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

Add escape key function on footer language/country selectors #148

Merged
merged 1 commit into from
Jul 13, 2021

Conversation

tauthomas01
Copy link
Contributor

Why are these changes introduced?

Fixes #73

The goal of this PR is to hide the footer country/language selectors when using "Escape" key.

What approach did you take?

  • Add key event to hide the country/language panel when we receive Escape key.

Demo links

Checklist

@tauthomas01 tauthomas01 requested a review from svinkle July 8, 2021 13:54
Copy link
Contributor

@ludoboludo ludoboludo left a comment

Choose a reason for hiding this comment

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

Works like a charm!

It's pretty close to what DetailsDisclosure is doing in details-disclosure.js but not sure how it could be reused. Maybe if LocalizationForm was extending from DetailsDisclosure 🤔 Not sure.

@tauthomas01
Copy link
Contributor Author

It's pretty close to what DetailsDisclosure is doing in details-disclosure.js but not sure how it could be reused. Maybe if LocalizationForm was extending from DetailsDisclosure 🤔 Not sure.

From the code, DetailsDisclosure uses <details><summary> elements, whereas LocalizationForm is using <ul> and <button>. However, I agree the functionalities look similar – the semantics are different so that must be the reason for separating the language/country selectors and details disclosure component.

@tauthomas01 tauthomas01 merged commit a188b1f into main Jul 13, 2021
@tauthomas01 tauthomas01 deleted the fix-footer-selectors-escape branch July 13, 2021 13:25
phapsidesGT pushed a commit to Gravytrain-UK/gt-shopify-dawn-theme that referenced this pull request Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Footer] Selectors do not support Esc key
3 participants