-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Notes: Add form submission shortcut #72868
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
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +62 B (0%) Total Size: 2.42 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 567cacf. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/19036878661
|
t-hamano
left a comment
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.
LGTM 👍
|
@Mamaduka looks like some merge conflicts but once resolved probably good to merge |
|
We might need to hold off on merging PRs related to feature enhancements that cannot be backported to the |
|
Great point @t-hamano, should we add a |
|
@Mamaduka, I've added the |
|
I'll merge this in RC cycle. Merge conflicts should be minimal then. |
567cacf to
c7df703
Compare
|
Rebased; going to merge after all checks are complete and green. |
|
@Mamaduka note that we'll hopefully have some more bug fixes to merge in and backport ahead of WP 6.9 RC1 tomorrow, so not sure if a slight delay on merging this PR until after RC1 is released may be best? |
|
Sure. I was at contributions day yesterday and remember this PR. It’s not time sensitive, will merge later. |
|
In discussing with @desrosj @adamsilverstein perhaps the polish here may be worth getting in for 6.9 and backporting before RC1? |
c7df703 to
6615c86
Compare
|
I rebased this branch to resolve the conflicts.
I believe backporting is possible if there is agreement among the contributors. This PR doesn't introduce a new API, but is simply an improvement to the user experience, so I think it should be possible. cc: 6.9 tech leads @dream-encode, @ellatrix , and @priethor for visibility. |
I'd like to get this in before RC1. As it's a major feature of 6.9, this could be considered a continuation task, so I see no issue with it coming in, even if it's ENH-adjacent. Would defer to my co-Tech Leads if they have any differing opinions. |
6615c86 to
987b962
Compare
|
The reasoning for RC1 inclusion makes sense. I've fixed the failing e2e test and this should be good to merge after all checks are green. |
Co-authored-by: Mamaduka <[email protected]> Co-authored-by: t-hamano <[email protected]> Co-authored-by: adamsilverstein <[email protected]> Co-authored-by: jeffpaul <[email protected]> Co-authored-by: dream-encode <[email protected]> Co-authored-by: karthick-murugan <[email protected]> Co-authored-by: desrosj <[email protected]>
|
I just cherry-picked this PR to the wp/6.9 branch to get it included in the next release: 2835cda |
What?
Closes #72721.
Alternative to #72864. I'm trying to use native form functionality.
PR adds keyboard shortcut for submitting the notes form - CMD + Enter (Mac) or CTRL + Enter (Windows).
Why?
Matches comment keyboard shortcut behavior in apps.
Testing Instructions
PR contains e2e test for the new behavior.
Testing Instructions for Keyboard
Same.