-
Notifications
You must be signed in to change notification settings - Fork 975
Escape key now closes Add Funds dialog #13884
Conversation
Codecov Report
@@ Coverage Diff @@
## master #13884 +/- ##
=========================================
- Coverage 56.54% 56.35% -0.2%
=========================================
Files 283 283
Lines 29036 29148 +112
Branches 4824 4828 +4
=========================================
+ Hits 16419 16426 +7
- Misses 12617 12722 +105
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works well for the payment tab, which satisfies the issue :) However, it does not work for the sync overlays. It seems it was intended to since onEsc
was added to preferenceTabs.SYNC
. So all you'd have to do is add checks for the syncOverlayVisible state attributes and it should work fine.
On another note, and probably out of scope for this issue, I have a feeling that some refactoring could be done for the overlays so the code/states could be a little more general, then we'd be able to tie 'esc' to close them in one place.
I agree. It's still a wip and there's something else going on in sync that isn't allowing it to work. Also, it's losing focus on some of the subdialogs if you click the button to go back and then try escape (example go to add funds, pick a currency, then click previous and esc no longer works until you click somewhere on the page. Sorry, I requested the review but forgot to mark it as WIP |
Escape key should now work with all the payments area. The Sync area is partially implemented with some areas losing focus. |
@jasonrsadler that's fine |
@@ -0,0 +1,49 @@ | |||
/* This Source Code Form is subject to the terms of the Mozilla Public |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this file is not needed here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. It's not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Open advance settings
- Open backup/recover dialog
- Press ESC
Advance settings dialog is closed and not backup/recover dialog
Can you try that again? On mine, when I go to advanced settings, click backup or recover, then press esc, the backup or recover dialog goes away. |
@jasonrsadler it's working now. The only thing that is not working now is
|
Yes. Forgot I left out the intro screen. I'll make sure it's included. |
lint Implemented escape for Add Funds modal overlay Refactored overlay code in escape sequence and also applied logic to sub linting. cherry pick on master rebase. Removed unused file Changed onBack to onNavigate. Added focuser for Add Funds batWelcome Implemented esc overlays on Sync
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test plan works well, sync dialogues are escaping fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++ works great
Escape key now closes Add Funds dialog
Escape key now closes Add Funds dialog
Resolves #3800
Submitter Checklist:
git rebase -i
to squash commits (if needed).Test Plan:
Reviewer Checklist:
Tests