Dialogs system implementation #15285
Conversation
…iews. Brought support for focus cycling integration with child views.
… characters with in-modal focus tracking.
… entire Dialog a DraggableView. Improved performance of the DraggableViewMixin.
…and simplify dialog creation for most integrators.
… ensure dialogs don't obscure the panel if not positioned manually.
packages/ckeditor5-find-and-replace/src/findandreplaceconfig.ts
Outdated
Show resolved
Hide resolved
Internal (ui): Fixed tab key cycling in a dialog that only has action buttons (no close button).
|
|
||
| /** | ||
| * Moves the dialog view to the off-screen position. | ||
| * Used when there's no space to display the dialog. |
There was a problem hiding this comment.
I don't understand this comment. It sounds like we move the dialog off screen when the dialog is too big for the viewport (no space). Did you mean it or did you mean that we move it off screen when there's no correct place to pin the dialog to (e.g. no root or something)?
|
|
||
| this.moveTo( leftCoordinate, domRootRect.top + defaultOffset ); | ||
| } else { | ||
| this._moveOffScreen(); |
There was a problem hiding this comment.
I am not sure about that 🤔, did we discuss it? When there can be no 🤔domRootRect (I can imagine no root in the editor, something else?) Then, if there's no domRootRect, shouldn't we use screen center or close it instead moving off screen?
I think I get it -- this can happen if the dialog is pinned to a part of the dom root and we scroll so the dom root is no longer visible, then you move the dialog off the screen. But in this case... does it really make sense? Why not keep the dialog where it was?
There was a problem hiding this comment.
It's consistent with other UI parts like dropdowns or balloons, but I don't really know if there's a reason for it - cc @oleq
| // This logic handles cases when textarea has been hidden while changes were made to its content but they could not | ||
| // be reflected to its height (auto-grown) because it was invisible or detached from DOM. The auto-grow logic | ||
| // gets executed as soon as the textarea becomes visible again. | ||
| this._resizeObserver = new ResizeObserver( this.element!, evt => { |
There was a problem hiding this comment.
I tried to figure out why we ended up with this solution and why we need ResizeObserver for this. It is used only for the case described in the comment, which made me even more confusing.
In principle, what we want to do is: when the textarea becomes visible, we want to auto-resize it (as it's value may have changed while it was not visible).
But why we need ResizeObserver for that? AFAIR -- we don't have a precise way to observe when given element becomes visible/hidden if it is done outside of editor's control. However, I assume that ResizeObserver detects that, because when something gets hidden/visible its size becomes 0/non-0.
Is this correct?
If yes, then please change the comment so it describes what we want to achieve in simpler words and why we need ResizeObserver for that.
There was a problem hiding this comment.
For the context - the root cause for the whole change was to simplify the code in AI Assistant (https://github.com/cksource/ckeditor5-commercial/pull/5832#discussion_r1426461711). We didn't want to make people use the additional listener to reset their components before dialog is hidden, and onHide effectively works as afterHide. Hence we're making the Textarea responsible for detecting when it can resize itself.
I added more elaborate comment explaining the generic issue.
Dumluregn
force-pushed
the
ck/14973-dialogs
branch
from
13cc648
to
15d6a26
Compare
…collection Other (ui): `FocusCycler`'s `#focusables` collection should only contain `FocusableView` instances. Closes #14973. MINOR BREAKING CHANGE (ui): The view collection `focusables` required by [`FocusCycler#constructor()`](https://ckeditor.com/docs/ckeditor5/latest/api/module_ui_focuscycler-FocusCycler.html#function-constructor) must only contain views implementing the [`FocusableView`](https://ckeditor.com/docs/ckeditor5/latest/api/module_ui_focuscycler-FocusableView.html) interface. Failing to do so will result in a TypeScript error. If your custom code creates `FocusCycler` instances, please make sure that all views passed in `focusables` implement the `focus()` method.
…if that's the only focusable view (#15623) Internal (ui): `FocusCycler#next` and `#previous` should point to the same view even if that's the only focusable view to allow for easier nested cyclers' integration. Previously they were set to `null`.
Co-authored-by: Szymon Cofalik <s.cofalik@cksource.com>
Suggested merge commit message (convention)
Feature (ui): Implemented the
Dialogplugin that allows for displaying dialog windows in the UI of the editor. Learn more about using dialogs. Closes #14973.Feature (find-and-replace): The UI of the feature has been migrated to a dialog for a better user experience. See #14973.
Fix (ui):
TextareaViewcomponent should update its size if its value has been changed while it was invisible. See #14973.Other (ui): The
FocusCycler#focusablescollection should only containFocusableViewinstances. See #14973.Other (ui): The
#nextand#previousproperties of aFocusCyclerinstance should point to the same view if there is only one focusable view registered in thefocusablescollection. See #14973.BREAKING CHANGE (find-and-replace): From this release on, the UI of the Find and replace feature is displayed by default in a dialog instead of a dropdown. To bring the previous user experience back, you can use the
config.findAndReplace.uiTypeconfiguration option. See #14973.MINOR BREAKING CHANGE (ui): The
--ck-z-modalCSS custom property has been renamed to--ck-z-panel. We recommend updating custom CSS and integrations that use this custom property to avoid presentation issues. See #14973.MINOR BREAKING CHANGE (find-and-replace): The layout of the UI has been changed. Please keep in mind that customizations based on certain CSS selectors may not work anymore because of a different DOM structure in the UI. Learn more about the scope of changes. See #14973.
MINOR BREAKING CHANGE (ui): The view collection (
focusables) required byFocusCycler#constructor()must only contain views implementing theFocusableViewinterface. Failing to do so will result in a TypeScript error. If your custom code createsFocusCyclerinstances, please make sure that all views passed infocusablesimplement thefocus()method. See #14973.MINOR BREAKING CHANGE (ui): The font size of the
FormHeaderViewcomponent has been increased. This change affects the look of Find and replace and Table styling features as well as custom user interfaces that use this component. See #14973.MINOR BREAKING CHANGE (ui): The type of
AriaLiveAnnouncerPolitenesshas been changed (previouslyenum, now a constantobject). See #14973.MINOR BREAKING CHANGE (ui): The
#nextand#previousproperties of aFocusCyclerwill now point to the same view if there is only one focusable view (previouslynull). This change may affect integrations that use this helper to manage advanced focus navigation in dynamic UIs. See #14973.Additional information
Requires https://github.com/cksource/ckeditor5-commercial/pull/5720.