Skip to content
This repository was archived by the owner on Apr 1, 2020. It is now read-only.

Conversation

feltech
Copy link
Contributor

@feltech feltech commented Jul 28, 2018

  • If the command menu is open and you alt-tab away from Oni, then
    alt-tab back again, the menu remains open but no longer has cursor
    focus.
  • So ensure focus is not lost on reactivating Oni by not overwriting the
    saved focus stack.

* If the command menu is open and you alt-tab away from Oni, then
alt-tab back again, the menu remains open but no longer has cursor
focus.
* So ensure focus is not lost on reactivating Oni by not overwriting the
saved focus stack.
@feltech
Copy link
Contributor Author

feltech commented Jul 28, 2018

This bug was annoying me when I needed to copy-paste text into the menu, but when I hit alt-tab back to Oni after copying the text, the menu was no longer selected. A minor annoyance, but it happened often enough to be worth looking into (and I figured it wouldn't be too difficult to fix).

As far as I can tell this little solution doesn't have any bad side-effects, but I wanted to PR this before starting on unit tests to get some feedback, since it has the potential to cause unexpected glitches if there's something I've missed, given that it's messing with cursor focus state.

@akinsho
Copy link
Member

akinsho commented Jul 28, 2018

@feltech this is amazing!! this is definitely one of my top 5 annoyances in Oni and it should fix #2071 will be sooooo good to get this in thanks 👍 💯

@codecov
Copy link

codecov bot commented Jul 29, 2018

Codecov Report

Merging #2472 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2472      +/-   ##
==========================================
- Coverage   43.88%   43.87%   -0.02%     
==========================================
  Files         343      343              
  Lines       13694    13695       +1     
  Branches     1800     1801       +1     
==========================================
- Hits         6010     6009       -1     
- Misses       7445     7447       +2     
  Partials      239      239
Impacted Files Coverage Δ
browser/src/Input/KeyboardInput.tsx 50.68% <100%> (ø) ⬆️
browser/src/Services/FocusManager.ts 55.55% <100%> (-9.16%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b921412...8eaaa72. Read the comment docs.

@feltech
Copy link
Contributor Author

feltech commented Jul 29, 2018

Provided this passes checks/review I think it's done. I've tried a few movements and no ill effects.

Copy link
Member

@akinsho akinsho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome 👍 thanks for the contribution @feltech

@akinsho akinsho merged commit 97f0c61 into onivim:master Jul 29, 2018
@akinsho
Copy link
Member

akinsho commented Jul 29, 2018

@feltech I've been running a build with this in today and only just realised that following this PR if you focus the explorer with <c-w-h> and then try and return to the buffer you are unable to enter any keys. I'm gonna make a PR to revert it I didn't see anything in the implementation that seemed like it would cause this 😢

@feltech
Copy link
Contributor Author

feltech commented Jul 29, 2018

Ah gutted. I was afraid of something like that. I tried a bunch of different things to test it, but clearly I got something wrong. I'm away for a week now, but when I return I'll try to get a better fix together.

akinsho added a commit to akinsho/oni that referenced this pull request Jul 29, 2018
akinsho added a commit that referenced this pull request Jul 30, 2018
Reverts #2472

@CrossR @bryphe this awesome bugfix PR unfortunately breaks a users ability to to navigate too and from the sidebar using keyboard bindings, in my excitement to have this annoying issue fixed I totally missed this bug till just now.
akinsho added a commit to akinsho/oni that referenced this pull request Jul 30, 2018
akinsho added a commit to akinsho/oni that referenced this pull request Jul 31, 2018
akinsho added a commit that referenced this pull request Aug 2, 2018
* Revert "Refocus previously open menu on reactivating Oni (#2472)"

This reverts commit 97f0c61.

* add initial git blame layer implementation

* make component absolute and pass position to component

* add cursor to layer context and use it render positional blame

* render blame inline if possible otherwise render above

* format blame date str and add format date to utils

* change ordering of blame message

* format time as time since and update styles for blame

* separate out inline and hover styles

* rename inline var

* add out of bounds checking

* add more error handling to out of bounds check dar

ken tooltip color

* darken text in blame

* fix lint error in blame layer

* fix existing broken tests

* add config options and experimental flag
add ability to run in manual mode require a binding to be hit to show the blame

* fix lint errors

* fix cursor line variants naming
add comments to explain, also fix out of bounds function

* fix comment change order of updateBlameCall

* use get derived props from state to prevent initial render flicker
pass in font-famil

* switch back to concurrently for start
query only a single line at a time and update state with next props

* remove unsed var in blame container

* prevent layer from rendering if vcs.blame is not enabled

* add recursive truncation of summary message

* add check if shortened all the way to the bottom just render the author and time

* fix comment typo

* only append ellipsis is shortened summary has content

* remove hover style as it is hard to manage improve fit checking

* position blame in first empty white space above is cant fit

* remove unused arguments

* increase default timeout, add exiting to transitionStyles

* move opacity in hope of making animation more pronounced

* check line position at start of reset timer matches current one before showing blame

* fix position related crash

* prettier fix

* mount the component ffs, add more tests

* moar test for blame layer

* fix failing test

* add overflow component and re-arrange comments

* fix lint errors

* add component did catch method
use words to calculate if truncation should happen
append only if truncated, use proper truncation symbol

* fix broken truncation assertion

* change check in git blame layer to use currentLine content

* fix configuration to use _oni.configuration
akinsho added a commit that referenced this pull request Aug 22, 2018
* add session manager, store and sidebar pane

* hook up session services to app

* move side effects into epics

* add creation and cancel creation actions

* add restore session epic and remove console.log

* await sessions in case of error

* add error handling to session functions

* Revert "Refocus previously open menu on reactivating Oni (#2472)"

This reverts commit 97f0c61.

* Revert "Refocus previously open menu on reactivating Oni (#2472)"

This reverts commit 97f0c61.

* remove console.log

* remove unused props passed to session pane

* add persist session command [WIP]

* Add current session action and update store with i

t

* use get user config add icons and some error handling

* add unclick handlers for sessions
move section title to general location
add delete session functionality

* add title and toggling of sessions add on vim leave handler

* fix lint errors

* refactor epics in preparation for 1.0 and rxjs 6

* update snapshot

* add bufdo bd command prior to restoring session
fix delete session bug causing reappearances

* create separate close all buffers method

* remove update session method

* Add audit time to update current session

* add close all buffers method and use persist session action in update session

* add restore session error action and complete action

* add console log for debugging return metadata directly

* add error handling to git blame function

* reduce persist audit time make ids in session.tsx readonly

* comment out console.log

* check neovim for current session when updating
this ensures the session is valid
added getCurrentSession method which checks vims v:this_session var

* fix lint errors

* add tests for sessions component and mock for neovim instance

* switch generic input check to specific text input view check

* add update timestamp functionality so these are more accurate

* switch to adding updated at by checking mtime of f

ile

* switch to storing sessions in persistent store

* add delete functionality to persistent store and mock

* fix lint error

* rename sessionName var to name for simplicity

* add session manager initial test

* create path using path.join

* add experimental config flag

* update Oni mock and increase sessionmanager tests

* add session store tests - [WIP]

* return simple session manager mock
remove need for instantiation, use done callback

* remove session store tests as redux observable api has changed
a large refactor of all observables is going to be required to upgrade redux observable so seems counter productive to write tests that will need to be re-written entirely once that is done

* add user configurable session directory

* tweak sidebar item styles

* update vim navigator to pass function to update selected item on click
render session sidebar item second

* fix lint error

* fix broken tests
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants