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

Format Library: Allow browser native undo/redo for link input #19488

Closed

Conversation

aduth
Copy link
Member

@aduth aduth commented Jan 7, 2020

Blocked by (currently merges to): #19487
Fixes #18755

This pull request seeks to allow for keyboard shortcuts for undo and redo to use the browser native behavior whilst adding or editing a link in a RichText field.

Implementation Notes:

Based on the previous discussion in #18755, I tried a few ideas for how to resolve this. Initially, I had hoped to use the same named shortcuts as the editor ("core/editor/undo" and "core/editor/redo") to ensure a consistent behavior against which to prevent the default. However, this would cause an implicit circular dependency between the format-library and editor modules.

The implementation here consolidates a bit of the key handling, where there is currently already handling to avoid ObserveTyping from taking effect in response to certain key events in the link editor.

I still think there might be some opportunity to generalize the behavior here, considering that we also have a similar need for the inline image width control (amongst others as well).

It might be something where we can devise some consistent condition for how these key events are to be excluded: For example, treating inputs within a popover (or modal, or dropdown) within the editor area as being exempt to consideration by ObserveTyping and/or the global editor history state keyboard shortcuts. However, at least in the latter case, it's not always clear if this is the expected behavior, in case those inputs interact directly with editor state. It's more likely to be a sufficient solution to address the need for ObserveTyping ignored events.

Testing Instructions:

Verify there is no regression in the behavior of viewing, adding, editing, or removing links.

Furthermore, verify that when editing a link and pressing Ctrl+Z or Ctrl+Shift+Z to undo or redo a change within the input, the change affects the input text and not the top-level editor.

@aduth aduth added [Type] Bug An existing feature does not function as intended [Package] Format library /packages/format-library [Feature] History History, undo, redo, revisions, autosave. labels Jan 7, 2020
@aduth
Copy link
Member Author

aduth commented Jan 28, 2020

The upstream pull request #19487 was closed in anticipation of a separate effort which would refactor the LinkControl component as a function component (#19462).

Technically, these changes don't require either #19487 or #19462. It could be rebased against the current master implementation if need-be.

@aduth
Copy link
Member Author

aduth commented May 25, 2020

This is still an active bug, but I'm not convinced this is the best solution to the broader problem, where I believe an approach like this may be necessary, but at the level of the popover component which should be treated as a separate event bubbling context.

The issue #18755 remains open for tracking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] History History, undo, redo, revisions, autosave. [Package] Format library /packages/format-library [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant