Skip to content

Conversation

@westonruter
Copy link
Member

@westonruter westonruter commented Feb 2, 2026

Trac ticket: https://core.trac.wordpress.org/ticket/17133

For details, see:


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@westonruter westonruter marked this pull request as draft February 2, 2026 23:54
westonruter and others added 6 commits February 2, 2026 15:59
Expose the `updateErrorNotice` method on the `wp.codeEditor` instance to allow forcing the display of linting errors.

In the Theme and Plugin editors, this method is now called during the submission process. This ensures that errors are visible when using the save shortcuts (`Ctrl+S` or `Cmd+S`) while the editor is focused, preventing a silent failure when the save is blocked by validation errors.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…ghlighting (CodeMirror) is disabled

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Updates the Code Editor to use the `inputRead` event instead of `keyup` for triggering autocompletion.

`inputRead` is more reliable than `keyup` for triggering autocompletion because it fires only when the document content has actually changed due to user input.

1.  **Modifier Keys Don't Trigger It:** `keyup` fires whenever *any* key is released. This includes `Shift`, `Ctrl`, `Cmd`, `Alt`, etc. This was causing a bug where releasing `Cmd` after `Cmd+S` triggered the `keyup` listener. `inputRead` ignores these entirely because pressing `Cmd` doesn't insert text into the editor.

2.  **No Race Conditions:** With `keyup`, one has to guess if the keystroke was part of a combo (like `Cmd+S`) by checking modifier states. However, if the user releases the modifier *before* the character key, the `keyup` event for the character key sees `metaKey: false`, looking exactly like a normal typed character. `inputRead` avoids this race condition completely because the save command (Cmd+S) doesn't insert text, so `inputRead` never fires.

3.  **Handles Non-Keyboard Input:** `inputRead` also handles cases like dragging and dropping text or using an Input Method Editor (IME), where `keyup` events can be complex.

4.  **Simpler Logic:** There is no need to manually filter out navigation keys (Arrows, Home, End) or function keys. `inputRead` only indicates "text was added", which is exactly when we want to consider showing a hint.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Fixes a `JQMIGRATE: jQuery.fn.submit() event shorthand is deprecated` warning by using the explicit `.trigger()` method to initiate form submission.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@westonruter

This comment was marked as outdated.

@github-actions
Copy link

github-actions bot commented Feb 3, 2026

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

westonruter and others added 2 commits February 2, 2026 21:21
Update the global keyboard listener for save shortcuts to use the same trigger mechanism as the CodeMirror shortcut handler.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Improves the robustness of the autocompletion trigger by specifically allowing only `+input` and `*compose` origins.

These origins are part of CodeMirror 5's internal system for categorizing document changes:
- `+input`: Represents direct character input from keyboard typing.
- `*compose`: Represents Input Method Editor (IME) composition, used for languages like Japanese, Chinese, or Korean.

By specifically targeting these origins, the editor effectively ignores changes that should not trigger autocompletion, such as `undo`, `redo`, `drag`, or programmatic updates. This also replaces the previous less-specific check for `paste`.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@westonruter
Copy link
Member Author

Review form Gemini:

Here is the final review of the changes, incorporating the latest robustness and consistency improvements.

Summary of Changes

The PR improves the WordPress Code Editor (used in Theme and Plugin editors) by making autocompletion significantly more robust and fixing issues with save shortcuts.

Key Technical Enhancements:

  1. Autocomplete Reliability: Switched from keyup to inputRead for triggering hints. This ensures that autocompletion only fires when content actually changes due to typing or IME composition (+input and *compose origins), effectively ignoring modifier key releases like the S in Cmd+S.
  2. Global Save Shortcut: Added a global keydown listener to the window in the Theme/Plugin editor. This ensures Ctrl+S and Cmd+S work even when the editor isn't focused (e.g., after dismissing a lint notice).
  3. Forced Error Visibility: Exposed updateErrorNotice on the wp.codeEditor instance. The Theme/Plugin editor now calls this on submit to ensure lint errors are visible if a save is blocked, preventing "silent" save failures.
  4. Modern Standards: Switched from deprecated jQuery .submit() shorthands to .trigger( 'submit' ).

Detailed Review

src/js/_enqueues/wp/code-editor.js

  • Robustness: The origin check '+input' !== change.origin && ! change.origin.startsWith( '*compose' ) is excellent. It targets the exact internal origins CodeMirror uses for typing, filtering out noise from paste, undo, redo, and drag.
  • Logic: The check for 1 !== change.text.length || 1 !== change.text[0].length correctly limits autocompletion to single-character typing, which is the intended behavior for the integrated linters.
  • Compatibility: Usage of startsWith is appropriate given the /* eslint-env es2020 */ header.
  • Documentation: JSDoc correctly updated for configureLinting and CodeEditorInstance.

src/js/_enqueues/wp/theme-plugin-editor.js

  • Consistency: The use of component.form.trigger( 'submit' ) in both the global window listener and the CodeMirror extraKeys handler is a significant improvement for consistency and reliability. It ensures the form's submit event is handled uniformly regardless of how it was triggered.
  • UX Fix: Forcing the error notice on submit resolves a common point of confusion where users couldn't see why their save was failing while focused in the editor.
  • Coding Standards: The code adheres to WordPress standards regarding indentation, Yoda conditions, and proper event handling.

Conclusion

The latest iteration addressed all previously identified nit-picks, including the "race condition" on shortcut release and the consistency of the submit trigger. The resulting code is clean, robust, and follows WordPress Core best practices.

Final Approval.

@westonruter westonruter marked this pull request as ready for review February 3, 2026 05:41
@github-actions
Copy link

github-actions bot commented Feb 3, 2026

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 props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props desrosj, westonruter, jonsurrell, mukesh27.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@westonruter westonruter requested a review from sirreal February 3, 2026 05:55
@sirreal
Copy link
Member

sirreal commented Feb 3, 2026

I noticed some strange behavior where lint errors (at least for JavaScript) seem to accumulate. I believe this is also present on trunk, so this is just an observation.

cumulative-errors.mov

Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

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

This seems to work well. I found several quirks in the editor, but they're all pre-existing on trunk.

Comment on lines +85 to 87
* Note that rules must be sent in the "deprecated" lint.options property
* to prevent linter from complaining about unrecognized options.
* See <https://github.com/codemirror/CodeMirror/pull/4944>.
Copy link
Member

Choose a reason for hiding this comment

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

I don't know exactly what's going on in this comment, but it caught my attention.

codemirror/codemirror5#4944 was merged so this may be out of date?

Not necessary to change anything in this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good callout. After merging this I intend to do an overall pass on this file and theme-plugin-editor.js to clean up stuff like this. I'm going to fix types and update docs. Once everything is pristine, then this should greatly help with plotting a course for v6.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements Ctrl/Cmd + S keyboard shortcuts to save files in the WordPress Theme and Plugin Editors. The implementation adds shortcuts both at the window level (for when syntax highlighting is disabled) and within CodeMirror (for when the editor has focus).

Changes:

  • Added Ctrl/Cmd + S keyboard shortcuts for file saving
  • Exposed updateErrorNotice function from code editor API
  • Refactored autocomplete trigger logic from keyup to inputRead event

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
src/js/_enqueues/wp/theme-plugin-editor.js Adds window-level and CodeMirror keyboard shortcuts for save, calls updateErrorNotice before submit
src/js/_enqueues/wp/code-editor.js Returns updateErrorNotice from configureLinting, refactors autocomplete from keyup to inputRead

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 418 to 421
editor.codemirror.setOption( 'extraKeys', {
'Ctrl-S': onSaveShortcut,
'Cmd-S': onSaveShortcut,
} );
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

Using setOption with extraKeys will completely replace the existing extraKeys configuration that was set during initialization. The default extraKeys include important shortcuts like Ctrl-Space for autocomplete, Ctrl-/ and Cmd-/ for toggleComment, and Ctrl-F/Cmd-F/Alt-F for findPersistent (see src/wp-includes/general-template.php line 4136). These will be lost when this code runs.

Instead, you should merge the new shortcuts with the existing ones. Get the current extraKeys first, then extend them with the new shortcuts.

Suggested change
editor.codemirror.setOption( 'extraKeys', {
'Ctrl-S': onSaveShortcut,
'Cmd-S': onSaveShortcut,
} );
var existingExtraKeys = editor.codemirror.getOption( 'extraKeys' ) || {};
var mergedExtraKeys = $.extend( {}, existingExtraKeys, {
'Ctrl-S': onSaveShortcut,
'Cmd-S': onSaveShortcut
} );
editor.codemirror.setOption( 'extraKeys', mergedExtraKeys );

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a great suggestion. Existing shortcuts were indeed being lost:

Shortcut Command
Alt-F findPersistent
Cmd-/ toggleComment
Cmd-F findPersistent
Ctrl-/ toggleComment
Ctrl-F findPersistent
Ctrl-Space autocomplete

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed in 2d71516

westonruter and others added 3 commits February 3, 2026 21:40
Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
pento pushed a commit that referenced this pull request Feb 4, 2026
* Keyboard shortcuts work when CodeMirror is not enabled (due to syntax highlighting not being enabled), and when the user is not focused inside the CodeMirror editor.
* The autocomplete trigger is switched from `keyup` to `inputRead` to improve reliability, support IME composition, and prevent conflicts with modifier keys (e.g., releasing `Ctrl`/`Cmd` before `s` after a save).
* A `updateErrorNotice` method is exposed on the code editor instance to ensure validation errors are displayed when a save via shortcut is attempted, preventing "silent" failures. Otherwise, the linting error notice is only shown when focus leaves the editor.
* The form submission is modernized by replacing the deprecated jQuery `.submit()` shorthand with `.trigger( 'submit' )`.

Developed in #10851

Props westonruter, Junaidkbr, evansolomon, desrosj, mukesh27, jonsurrell, spiraltee, chexee, andrewryno, tusharaddweb, gauri87, huzaifaalmesbah, ocean90, karmatosed, johnbillion, scribu, jcnetsys.
Fixes #17133.


git-svn-id: https://develop.svn.wordpress.org/trunk@61588 602fd350-edb4-49c9-b593-d223f7449a82
@github-actions
Copy link

github-actions bot commented Feb 4, 2026

A commit was made that fixes the Trac ticket referenced in the description of this pull request.

SVN changeset: 61588
GitHub commit: 845f7ce

This PR will be closed, but please confirm the accuracy of this and reopen if there is more work to be done.

@github-actions github-actions bot closed this Feb 4, 2026
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Feb 4, 2026
* Keyboard shortcuts work when CodeMirror is not enabled (due to syntax highlighting not being enabled), and when the user is not focused inside the CodeMirror editor.
* The autocomplete trigger is switched from `keyup` to `inputRead` to improve reliability, support IME composition, and prevent conflicts with modifier keys (e.g., releasing `Ctrl`/`Cmd` before `s` after a save).
* A `updateErrorNotice` method is exposed on the code editor instance to ensure validation errors are displayed when a save via shortcut is attempted, preventing "silent" failures. Otherwise, the linting error notice is only shown when focus leaves the editor.
* The form submission is modernized by replacing the deprecated jQuery `.submit()` shorthand with `.trigger( 'submit' )`.

Developed in WordPress/wordpress-develop#10851

Props westonruter, Junaidkbr, evansolomon, desrosj, mukesh27, jonsurrell, spiraltee, chexee, andrewryno, tusharaddweb, gauri87, huzaifaalmesbah, ocean90, karmatosed, johnbillion, scribu, jcnetsys.
Fixes #17133.

Built from https://develop.svn.wordpress.org/trunk@61588


git-svn-id: http://core.svn.wordpress.org/trunk@60899 1a063a9b-81f0-0310-95a4-ce76da25c4cd
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.

4 participants