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 integration for fullscreen feature with revision history #17941

Open
wants to merge 1 commit into
base: ck/fullscreen-bootstrap
Choose a base branch
from

Conversation

Dumluregn
Copy link
Contributor

@Dumluregn Dumluregn commented Feb 17, 2025

Suggested merge commit message (convention)

Internal (fullscreen): Add integration with revision history.

Fix (ui): Do not open disabled menu bar menu on arrow down press. Closes #17915.

Feature (ui): Add MenuBarView#disable() and MenuBarView#enable() methods. They disable/enable all top-level menus in menu bar. Closes #17940.

Internal (ui): Add a toolbar property to the EditorUIView class for coherent access for all editor types.

Internal (ui): Make EditorUI#_initMenuBar() method public and rename it accordingly to allow for runtime toolbar creation.


Additional information

  • Requires ck/fullscreen-revision-history in the related repository (one of the used types needs to be exported).
  • Disabling menu bar was supposed to be done through a flag. However looking at API I proposed to use disable() and enable() methods instead as it will be simpler and more coherent with the rest of the component code.
  • Following the idea to have as little code in separate editor handlers (Fullscreen mode feature bootstrap #17897 (comment)), most of the RH integration was put in the AbstractEditorHandler class. It should be treated as a go-to class, not the one that only throws errors for unsupported editor types.
  • I added a RevisionHistoryMock class in unit tests providing as much integration as I found useful. It's arbitrary and I'm open for suggestions.
  • EditorUIView#toolbar was introduced to make the editor types UIs more coherent and better accessible. Without it here we'd have to use as keyword to narrow the editor types used in AbstractEditorHandler.
  • As discussed, for now the CC for revision history integration implementation details is skipped. We're waiting for some decisions regarding how to approach the problem of testing premium features integration in the public repo.

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.

1 participant