Skip to content
This repository has been archived by the owner on Sep 2, 2023. It is now read-only.

[FEATURE] Dismiss locale popup on Esc (RT-3280) #2402

Merged
merged 1 commit into from
May 11, 2015
Merged

[FEATURE] Dismiss locale popup on Esc (RT-3280) #2402

merged 1 commit into from
May 11, 2015

Conversation

mynetx
Copy link
Contributor

@mynetx mynetx commented Apr 6, 2015

Use rp-popup(rp-popup-dismiss-on-esc) attribute to allow dismissing the popup when the user presses the [Esc] key while the popup is shown.

@vhpoet
Copy link
Contributor

vhpoet commented Apr 6, 2015

@jublo can we just make this a default?

@mynetx
Copy link
Contributor Author

mynetx commented Apr 6, 2015

@vhpoet Sure—if there aren’t any popups that needs to run custom code on cancel?

@vhpoet
Copy link
Contributor

vhpoet commented Apr 6, 2015

esc should call the same function as cancel

@mynetx
Copy link
Contributor Author

mynetx commented Apr 6, 2015

so esc should just call popup.cancel()?

@rht
Copy link
Contributor

rht commented Apr 6, 2015

unlock.jade popup has esc as cancel, but with different implementation.

@mynetx
Copy link
Contributor Author

mynetx commented Apr 6, 2015

From my checks, these are the only 2 occurrences of the popup, so can I generalize the ESC button press as default for both of these?

@vhpoet
Copy link
Contributor

vhpoet commented Apr 6, 2015

yep, esc should call popup.cancel

Popup is dismissed when the user presses the [Esc] key while the popup
is shown. This works for `rp-popup` directives only, not if the popup
service is invoked directly.
@mynetx
Copy link
Contributor Author

mynetx commented Apr 7, 2015

popup.cancel() is not a function defined in the popup service. Pressing ESC now cancels any popup that was created with a rp-popup directive (thus excluding the unlock popup, which is invoked directly using the service, but that has its own ESC code).

@vhpoet
Copy link
Contributor

vhpoet commented May 11, 2015

LGTM

1 similar comment
@mrajvanshy
Copy link
Contributor

LGTM

mrajvanshy added a commit that referenced this pull request May 11, 2015
[FEATURE] Dismiss locale popup on Esc (RT-3280)
@mrajvanshy mrajvanshy merged commit 31f287a into ripple:develop May 11, 2015
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.

4 participants