refactor!: update crud editor dialog to use native popover #9790
Conversation
| /** @protected */ | ||
| updated(props) { | ||
| super.updated(props); | ||
|
|
||
| // When using dialog mode, hide elements not slotted into the dialog from accessibility tree | ||
| if ( | ||
| props.has('_grid') || | ||
| props.has('_newButton') || | ||
| props.has('editorOpened') || | ||
| props.has('editorPosition') || | ||
| props.has('_fullscreen') | ||
| ) { | ||
| const hide = this.editorOpened && this._dialogMode; | ||
| this.__hideElement(this._grid, hide); | ||
| this.__hideElement(this._newButton, hide); | ||
| } | ||
| } |
There was a problem hiding this comment.
The aria-hidden logic needs some customization, as we need to hide all elements that are not in the dialog. So the solution now is:
- Use CRUD itself as modal root
- Then add this custom logic to also hide elements that are not slotted into the dialog when it is opened
| // Wait to set label until header node has updated (observer seems to run after this one) | ||
| setTimeout(() => { | ||
| this.__dialogAriaLabel = this._headerNode.textContent.trim(); | ||
| }); |
There was a problem hiding this comment.
This could be better, but I couldn't find a simple solution.
Updating the label here is easiest as it also handles text content of custom header nodes.The timeout could be avoided if all observer logic were moved into updated. Then we could control the order of updates, so that the default header node would be updated before running this opened update logic. But that would also require several changes.
| </div> | ||
| <slot name="form"></slot> | ||
| </div> | ||
| ${!this._dialogMode |
There was a problem hiding this comment.
Considered using cache as suggested, but I think switching between modes shouldn't happen as often (or at all for a single user), so I figure it's not necessary.
| afterEach(async () => { | ||
| // Wait until the crud dialog overlay is closed | ||
| let overlay; | ||
| while ((overlay = document.querySelector('body > vaadin-crud-dialog-overlay'))) { | ||
| // Press esc to close the dialog | ||
| overlay.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true })); | ||
| await aTimeout(1); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Didn't seem to be necessary anymore?
| ['default', 'custom'].forEach((type) => { | ||
| describe(`${type} form`, () => { | ||
| let dialog, form, btnCancel, overlay; | ||
| describe('editor mode', () => { |
There was a problem hiding this comment.
Replaced the previous teleporting logic tests with this suite that verifies that either the inline editor or dialog are rendered with respective slots.
|
|
This ticket/PR has been released with Vaadin 25.0.0-beta2. |
3 participants
Description
Fixes #9732
Type of change