Skip to content
This repository was archived by the owner on May 1, 2025. It is now read-only.

Dialog fixes#508

Merged
itamargiv merged 10 commits into
masterfrom
dialog-fixes
Dec 7, 2021
Merged

Dialog fixes#508
itamargiv merged 10 commits into
masterfrom
dialog-fixes

Conversation

@guergana
Copy link
Copy Markdown
Contributor

@guergana guergana commented Dec 2, 2021

This change fixes several issues with the Dialog component, and it's story:

  1. The Dialog overlay did not cover the whole viewport/iframe - This was due to some copying and pasting error where the positioning of the dialog within the viewport was reverted from 50%, 50% (x, y) to 0, 0; and a transform was introduced to try and compensate for it. To fix, the transform was removed and the original positioning values restored.

  2. The Scrollbars were not restored in storybook, while navigating from the dialog story when the dialog was still open - This was actually two separate issues:
    A. The dialog itself did not complete the hiding animation, and thus the _restoreScroll method was not called. This was mitigated by adding a direct call to _restoreScroll to the beforeDestroy lifecycle hook.
    B. Storybook does not destroy vue components when switching from the canvas tab to the docs tab (see filed bug on storybook). While this wasn't fixed, this was worked around by ensuring the scrolling is only trapped and restored when the window actually has scrollbars.

  3. The Icon for the close button was replaced.

Additionally, minor fixes included in this change are: passing the correct boolean type (instead of string) as the dialog's dismiss-button prop, and changing the dialog's viewport height from a fixed height (which caused the docs tab to add extra space to the component), to a max-height.

Bug: T297093

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Dec 2, 2021

@sai-san
Copy link
Copy Markdown
Contributor

sai-san commented Dec 3, 2021

Listing the needed fixes here:

  • Storybook scroll: The incorporation of the Dialog component to Storybook as an outcome of Add Story & Storybook Documentation for Dialog Component #507 has broken the scroll in all Storybook pages
  • Overlay: the overlay position needs correction, as it should be centered and fully cover the page/wrapper containing the Dialog
  • Close button: The icon inside the Dialog's close button should be size="large"

Comment thread vue-components/src/components/Dialog.vue Outdated
Copy link
Copy Markdown
Member

@Silvan-WMDE Silvan-WMDE left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and works 👍

@itamargiv itamargiv merged commit 6417714 into master Dec 7, 2021
@itamargiv itamargiv deleted the dialog-fixes branch December 7, 2021 09:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants