Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ContextKeys are missing and not properly defined #2428

Closed
fanantoxa opened this issue Sep 9, 2020 · 2 comments
Closed

ContextKeys are missing and not properly defined #2428

fanantoxa opened this issue Sep 9, 2020 · 2 comments
Labels
A-ux Area: Overall user experience and aesthetics

Comments

@fanantoxa
Copy link
Contributor

Current implementation of src\Model\ContextKeys.re not fully correct

It missing both textInputFocus and menuFocus
The were removed here #786 but wasn't added back anywhere.

Also after looking in ContextKeys.re I've noticed that other key also not propely defined.
For example editorTextFocus

| TerminalInsert
| TerminalNormal
| TerminalVisual(_) => false
| _ => true

Basically it say that it on focus all the time but not terminal.
And there kinda wrong because you might be in Menu, Search, WildMenu, and other places at this moment.

That why paste into text input currently work in insertMode and also work when I add editorTextFocus

Same goes for terminalFocus
is active when

| TerminalInsert
| TerminalNormal
| TerminalVisual(_) => true
| _ => false

But still can open menu or smth.

Can we refine and refactor this to be more persistent and clear about different ContextKeys?
It already blocking me from further improving textInputs and few other things.
In addition we have Keybinding Suggestions and Contribution Opportunities
as good-first-issue and people going to be confused while working on it.

bryphe added a commit that referenced this issue Sep 11, 2020
…fixes (#2433)

This is more groundwork being laid to support #528 , which requires us to refine our focus management story. This enables features to conditional expose context keys (which may be from nested items - like the sidebar exposing `textInputFocus` when an input control has focus in a sidebar item...)

This moves `InputText` to a component - a rich component that exposes context keys, potentially keybindings, subscriptions, etc. I'll follow this same model for some additional rich components I'll be adding to support #528 

I started a debug view for input / context keys in #2427 , it's ugly right now - but it shows the `textInputFocus` key being set as expected in various scenarios:

![2020-09-10 15 32 01](https://user-images.githubusercontent.com/13532591/92817678-aef46280-f37b-11ea-82d7-fb5c4046c203.gif)

This fixes the `textInputFocus` part of #2428 , and requires a revery fix for drop-shadow to give more emphasis around text-inputs having focus: revery-ui/revery#999
@bryphe bryphe added the A-ux Area: Overall user experience and aesthetics label Sep 11, 2020
bryphe added a commit that referenced this issue Sep 17, 2020
This fixes the `textInputFocus` and `editorTextFocus` context keys per #2428
@fanantoxa
Copy link
Contributor Author

@bryphe I see 2 PR that fix this issue. Nice work!

@bryphe
Copy link
Member

bryphe commented Sep 22, 2020

Awesome! Thanks @fanantoxa for confirming, appreciate the help

I added a (ugly) debug UI to help troubleshoot these sorts of issues as well:

Uploading 2020-09-22 15.44.59.gif…

Let me know if you see any other context keys that are incorrect 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ux Area: Overall user experience and aesthetics
Projects
None yet
Development

No branches or pull requests

2 participants