- 
                Notifications
    You must be signed in to change notification settings 
- Fork 87
refactor!: update dialog to use native popover #9807
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
Changes from 14 commits
d0ad5b6
              b14202a
              f210c18
              269b34c
              978792f
              3538645
              f122377
              a346564
              cbdb844
              172c87f
              6f54571
              e6a433b
              08b17d5
              8224076
              826eaa8
              a00e6d9
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -77,13 +77,12 @@ export const DialogBaseMixin = (superClass) => | |
| }, | ||
|  | ||
| /** | ||
| * The `role` attribute value to be set on the overlay. Defaults to "dialog". | ||
| * The `role` attribute value to be set on the dialog. Defaults to "dialog". | ||
|         
                  sissbruecker marked this conversation as resolved.
              Show resolved
            Hide resolved | ||
| * | ||
| * @attr {string} overlay-role | ||
| */ | ||
| overlayRole: { | ||
| type: String, | ||
| value: 'dialog', | ||
| }, | ||
| }; | ||
| } | ||
|  | @@ -103,6 +102,19 @@ export const DialogBaseMixin = (superClass) => | |
| overlay.addEventListener('vaadin-overlay-closed', this.__handleOverlayClosed.bind(this)); | ||
|  | ||
| this._overlayElement = overlay; | ||
|  | ||
| if (!this.hasAttribute('role')) { | ||
| this.role = 'dialog'; | ||
| } | ||
| } | ||
|  | ||
| /** @protected */ | ||
| updated(props) { | ||
| super.updated(props); | ||
|  | ||
| if (props.has('overlayRole')) { | ||
| this.role = this.overlayRole || 'dialog'; | ||
| } | ||
| } | ||
| 
      Comment on lines
    
      +106
     to 
      118
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed this so you can use  | ||
|  | ||
| /** @private */ | ||
|  | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -48,16 +48,13 @@ export { DialogOverlay } from './vaadin-dialog-overlay.js'; | |
| * | ||
| * ### Styling | ||
| * | ||
| * `<vaadin-dialog>` uses `<vaadin-dialog-overlay>` internal | ||
|         
                  sissbruecker marked this conversation as resolved.
              Show resolved
            Hide resolved | ||
| * themable component as the actual visible dialog overlay. | ||
| * | ||
| * See [`<vaadin-overlay>`](#/elements/vaadin-overlay) documentation. | ||
| * for `<vaadin-dialog-overlay>` parts. | ||
| * | ||
| * In addition to `<vaadin-overlay>` parts, the following parts are available for styling: | ||
| * The following shadow DOM parts are available for styling: | ||
| * | ||
| * Part name | Description | ||
| * -----------------|------------------------------------------- | ||
| * `backdrop` | Backdrop of the overlay | ||
| * `overlay` | The overlay container | ||
| * `content` | The overlay content | ||
| * `header` | Element wrapping title and header content | ||
| * `header-content` | Element wrapping the header content slot | ||
| * `title` | Element wrapping the title slot | ||
|  | @@ -108,32 +105,25 @@ class Dialog extends DialogSizeMixin( | |
|  | ||
| static get styles() { | ||
| return css` | ||
| :host { | ||
| :host, | ||
| [hidden] { | ||
| display: none !important; | ||
| } | ||
| `; | ||
| } | ||
|  | ||
| static get properties() { | ||
| return { | ||
| /** | ||
| * Set the `aria-label` attribute for assistive technologies like | ||
| * screen readers. An empty string value for this property (the | ||
| * default) means that the `aria-label` attribute is not present. | ||
| */ | ||
| ariaLabel: { | ||
| type: String, | ||
| value: '', | ||
| }, | ||
| }; | ||
| :host([opened]), | ||
| :host([opening]), | ||
| :host([closing]) { | ||
| display: contents !important; | ||
| } | ||
| `; | ||
| } | ||
|  | ||
| /** @protected */ | ||
| render() { | ||
| return html` | ||
| <vaadin-dialog-overlay | ||
| id="overlay" | ||
| role="${this.overlayRole}" | ||
| popover="manual" | ||
| .owner="${this}" | ||
| .opened="${this.opened}" | ||
| .headerTitle="${this.headerTitle}" | ||
|  | @@ -144,15 +134,46 @@ class Dialog extends DialogSizeMixin( | |
| @mousedown="${this._bringOverlayToFront}" | ||
| @touchstart="${this._bringOverlayToFront}" | ||
| theme="${ifDefined(this._theme)}" | ||
| aria-label="${ifDefined(this.ariaLabel || this.headerTitle)}" | ||
| .modeless="${this.modeless}" | ||
| .withBackdrop="${!this.modeless}" | ||
| ?resizable="${this.resizable}" | ||
| restore-focus-on-close | ||
| focus-trap | ||
| ></vaadin-dialog-overlay> | ||
| exportparts="backdrop, overlay, header, title, header-content, content, footer" | ||
| > | ||
| <slot name="title" slot="title"></slot> | ||
| <slot name="header-content" slot="header-content"></slot> | ||
| <slot name="footer" slot="footer"></slot> | ||
| <slot></slot> | ||
| </vaadin-dialog-overlay> | ||
| `; | ||
| } | ||
|  | ||
| /** @protected */ | ||
| updated(props) { | ||
| super.updated(props); | ||
|  | ||
| if (props.has('headerTitle')) { | ||
| this.__applyHeaderTitleAriaLabel(); | ||
| } | ||
| } | ||
|  | ||
| /** @private */ | ||
| __applyHeaderTitleAriaLabel() { | ||
| // If there is a header title and no aria-label, | ||
| // set the aria-label to the header title. | ||
| if (this.headerTitle && !this.ariaLabel) { | ||
| this.ariaLabel = this.headerTitle; | ||
| this.__useHeaderTitleAriaLabel = true; | ||
| } | ||
|  | ||
| // If there is no header title and the aria-label was set | ||
| // using a header title, remove the aria-label. | ||
| if (this.__useHeaderTitleAriaLabel && !this.headerTitle) { | ||
| this.ariaLabel = null; | ||
| this.__useHeaderTitleAriaLabel = false; | ||
| } | ||
| } | ||
|          | ||
| } | ||
|  | ||
| defineCustomElement(Dialog); | ||
|  | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessary anymore, as it's set by
DialogBaseMixin