Skip to content
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

Edit Post: Fix Welcome Guide modal display for Internet Explorer #19201

Merged
merged 4 commits into from
Dec 18, 2019

Conversation

aduth
Copy link
Member

@aduth aduth commented Dec 17, 2019

Fixes #19140

This pull request seeks to resolve a number of issues with the Welcome Guide as it is displayed in Internet Explorer. Most importantly, the modal can now be dismissed.

Before After
Before After

These fixes include:

  • Text would overflow in the modal due a Flexbox bug associated with Internet Explorer and the use of align-items: center on Flex containers (reference). Since this style did not appear to have a noticeable impact on the display of the modal, it was simply removed.
  • The "Close" button could not be clicked because it relied on a z-index styling to appear "above" the rest of the content in the modal. However, since z-index only applies to positioned elements (reference), it was not taking effect, because the only positioning applied to the element was position: sticky, not supported in Internet Explorer (reference). This was resolved by applying position: relative as a "fallback" positioning, allowing the z-index to take effect as expected.
  • The image was intended to appear as vertically centered using absolute positioning. There were no explicit styles used to actually apply the centering effect. While these did not seem to be necessary for other browsers, the explicit centering was introduced to ensure that the image is centered correctly in Internet Explorer.
  • The "Finish" button would previously not appear in Internet Explorer, because it was expected to take effect using display: unset, a value unsupported in Internet Explorer (reference). Instead, display: block is used, derived from the effective ("computed") value otherwise expected from display: unset.

This does not include handling to allow the modal background to be clicked to dismiss the guide. I expect this may be a separate issue with the Modal component.

Additionally, I was tempted to remove this bit of styling:

margin-top: -$header-height;

...since it seems in contradiction with how a modal is expected to be styled, and assumes some vertical centering of the modal contents which otherwise isn't enforced (for Welcome modal, it is applied in the welcome modal styling). Since these were not directly relevant for the Internet Explorer fix, I opted to leave them for separate revision.

Testing Instructions:

Repeat testing instructions from #18041, in Internet Explorer and in your preferred browser.

@aduth aduth added [Type] Bug An existing feature does not function as intended Browser Issues Issues or PRs that are related to browser specific problems [Feature] NUX Anything that impacts the new user experience labels Dec 17, 2019
@aduth aduth requested a review from noisysocks December 17, 2019 16:45
// For z-index to take effect, the element must be positioned. A "sticky"
// element is positioned, but since this is not supported in IE11,
// "relative" is used as a fallback.
position: relative;
Copy link
Contributor

@gwwar gwwar Dec 17, 2019

Choose a reason for hiding this comment

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

As an FYI if folks need a stacking context, any of the other rules on https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Positioning/Understanding_z_index/The_stacking_context will also do the trick, in case changing position has other side effects folks want to avoid.

https://developer.mozilla.org/en-US/docs/Web/CSS/isolation for example only does this, but it's unsupported by IE.

Copy link
Contributor

@gwwar gwwar left a comment

Choose a reason for hiding this comment

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

Thanks @aduth ! This tests well for me, code looks reasonable and works well on IE11 and Chrome.

fixed

@aduth aduth merged commit 5957797 into master Dec 18, 2019
@aduth aduth deleted the fix/19140-ie-modal branch December 18, 2019 15:11
@youknowriad youknowriad added this to the Gutenberg 7.2 milestone Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Browser Issues Issues or PRs that are related to browser specific problems [Feature] NUX Anything that impacts the new user experience [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot Dismiss Welcome Modal in IE11
3 participants