Fullscreen guide and minor fixes #18138
Conversation
0e0e25e to
794be40
Compare
794be40 to
ddd6dd0
Compare
…in fullscreen mode.
…rce-editing-document-outline
Feature (ui): Add a possibility to switch toolbar behavior in the runtime using the `ToolbarView#switchBehavior()` method. Internal (fullscreen): Allow to configure toolbar behavior while in fullscreen mode.
packages/ckeditor5-fullscreen/docs/_snippets/features/fullscreen-default.js
Outdated
Show resolved
Hide resolved
| definitions: [ | ||
| { | ||
| title: 'Document with an image', | ||
| description: 'Simple heading with text and and and image.', |
packages/ckeditor5-fullscreen/docs/_snippets/features/fullscreen-pageless.html
Outdated
Show resolved
Hide resolved
| @@ -182,8 +204,15 @@ export default class AbstractEditorHandler { | |||
| this._overrideRevisionHistoryCallbacks(); | |||
| } | |||
|
|
|||
| // Hide all other elements in the container to ensure they don't create an empty unscrollable space. | |||
There was a problem hiding this comment.
Why would there be anything in the container? Is this to account some mistake on the integrator part? Like e.g. whitespace text nodes or something? If so, shouldn't we remove them instead?
There was a problem hiding this comment.
For example the pageless demo in docs uses the container where there is the whole rest of website content and we need to hide it. We can't just fix the position because the rest of the page (outside the container) has to stay usable.
…f body collection elements.
|
I build docs and I see some things that we could improve. Maybe some of them are due to some building errors, IDK: Something is wrong with paddings here with the table: I wonder if it is good that we don't add any even slight padding to the sides of the container. The button is almost next to the screen edge, it looks and feels a bit weird: Compare to how far from the screen are GDocs buttons (I understand they have a bit different layout, but the point is to make it simpler to click toolbar/menubar buttons). When I leave fullscreen mode, the page is scrolled to the top -- the previous scrolled position is not preserved. On a big screen, the pageless demo looks weird. It is moved to the left and still has some grayish background on the right. Instead, I'd make the background-background white and I guess still probably centered 🤔. I am experiencing some warnings errors when building docs, e.g.: The doc page looks like it has incorrect layout, or IDK something went wrong: It seems that it uses the wide layout, like on Examples / Document editor, but the editors are not full-wide, so it looks like something is broken. I propose to either use regular layout (I understand that might be a problem for showcasing custom container?) or have the editor full-wide? I am also missing a sample where the basic editor (non-fullscreen) would be quite small, small editable size, no menu, maybe inline annotations? So that the fullscreen experience would feel more transformative. Sidebar has no margin between the editing area: |
Co-authored-by: Szymon Cofalik <s.cofalik@cksource.com>
Co-authored-by: Dawid Kossowski <20561161+DawidKossowski@users.noreply.github.com>
…document-outline Internal (fullscreen): Improve source editing integration with document outline in fullscreen mode.
Internal (fullscreen): Integrate with CKBox. Closes #18189. Internal (fullscreen): Fix dialogs and balloons positioning in scrollable container. Closes #18153. Internal (fullscreen): Remove icon and rename fullscreen menu bar. Closes #18187. Internal (fullscreen): Rename fullscreen toolbar buttons to "Enter/leave fullscreen mode". Closes #18188.
|
List of docs fixes left to do (I'll extract it to the ticket):
|
Suggested merge commit message (convention)
Docs (fullscreen): Add feature guide.
Feature (ui): Add a possibility to switch toolbar behavior in the runtime using the
ToolbarView#switchBehavior()method.Internal (fullscreen): Allow to configure toolbar behavior while in fullscreen mode.
Additional information
Checkout the same branch in commercial repo.
Part regarding the toolbar behavior switching was already reviewed in #18160.
Renames:
fullscreen->toggleFullscreento follow the convention of starting command names with a verb.fullscreen-onandfullscreen-off->fullscreen-disableandfullscreen-enableto avoid confusion.AbstractHandler#_container->AbstractHandler#_wrapperto avoid confusion with a configurable custom container.Refactors:
enable()/disable()methods for consistency, readability and easier testing.Fixes:
this._document = this._editor.sourceElement!.ownerDocument;turned out to not be reliable in some cases (e.g. pagination manual test. Fall back to the global document ifsourceElement` is not defined.editor.destroy(), disable the fullscreen only if wrapper is visible.