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

feat(lib): add event listener for ESC key handler #404

Merged
merged 8 commits into from
Sep 25, 2024

Conversation

Dawntraoz
Copy link
Contributor

What?

Whenever a modal is opened inside a custom field plugin, it is impossible to close the modal on escape. This keyboard listener fixes that issue inside and outside the iframe by changing the isModalOpen state and notifying the container.

Why?

JIRA: SHAPE-2151

This is important for people who use a keyboard only to navigate.

How to test? (optional)

  1. Run any template locally
  2. Go to the field-plugin editor at Partner Portal (Enable local dev mode)
  3. Open the modal, put the focus on something inside the iframe
  4. Press ESC, see the modal closing

@Dawntraoz Dawntraoz added bug Something isn't working enhancement New feature or request labels Sep 23, 2024
@Dawntraoz Dawntraoz self-assigned this Sep 23, 2024
Copy link

vercel bot commented Sep 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
plugin-sandbox ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 25, 2024 0:54am

@Dawntraoz
Copy link
Contributor Author

Working on the unit tests right now, but would love to have some review of the approach also @BibiSebi @eunjae-lee @demetriusfeijoo 💜

@eunjae-lee
Copy link
Contributor

I wonder if this will affect the existing plugins (once they upgrade the field plugin library version)

@Dawntraoz
Copy link
Contributor Author

I wonder if this will affect the existing plugins (once they upgrade the field plugin library version)

The idea was to update one of ours to try out after this PR is merged, but in my mind, that's actually what I tried to do putting it in the lib and not in the templates, I did right then?

Copy link
Contributor

@eunjae-lee eunjae-lee left a comment

Choose a reason for hiding this comment

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

Yeah I think this implementation makes sense :)
I've also left a comment on the storyfront PR.

@eunjae-lee
Copy link
Contributor

just realized that Alba is not included as a contributor :)

@all-contributors please add @Dawntraoz for code.

Copy link
Contributor

@eunjae-lee

I've put up a pull request to add @Dawntraoz! 🎉

@Dawntraoz
Copy link
Contributor Author

just realized that Alba is not included as a contributor :)

@all-contributors please add @Dawntraoz for code.

How cool is that bot 🤩 I didn't know it exists, you just made my day @eunjae-lee

Copy link
Contributor

@Dawntraoz

@Dawntraoz already contributed before to code

Copy link
Contributor

@demetriusfeijoo demetriusfeijoo left a comment

Choose a reason for hiding this comment

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

Great job @Dawntraoz, love it! 💯 💪

@Dawntraoz Dawntraoz merged commit 4279218 into main Sep 25, 2024
4 checks passed
@Dawntraoz Dawntraoz deleted the feat/SHAPE-2151-modal-close-handle branch September 25, 2024 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants