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

Fix/ie11 interface bugs #26944

Merged
merged 3 commits into from
Nov 17, 2020

Conversation

ambienthack
Copy link
Contributor

@ambienthack ambienthack commented Nov 12, 2020

Description

Fixes #26932 IE11 bugs:

  • first commit: made breadcrumbs show up in footer
  • second commit: made block editor fit into browser window

How has this been tested?

Tested on Windows 10: IE11, Chrome 86, Opera (latest), Firefox (latest)
IOS 14: Chrome, Safari

Screenshots

Screenshot_6

Types of changes

Footer CSS (.interface-interface-skeleton__footer):

  1. Made footer absolutely positioned istead if fixed positioned. Beacuse it is inside fixed positioned container, this shouldn't break anything.
  2. Added left:0 to the footer because in non-IE browsers the footer was positioned correctly in horizontal direction during static positioning stage. IE static positioning stage doesn't bring the footer to left:0.

Editor CSS (.interface-interface-skeleton__editor):
max-width: 100% => width: 100%;
max-width doesn't work with IE flexbox in some cases.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Nov 12, 2020
@gziolo gziolo added the Browser Issues Issues or PRs that are related to browser specific problems label Nov 13, 2020
@jasmussen
Copy link
Contributor

Thank you for the PR! And thank you for tackling this specific issue.

The PR touches very sensitive CSS code that carries the interface. So furthermore I appreciate that your PR is very small with just two small code changes. The thing that's tricky here is that your changes affect every browser, not just IE11, so it's important to test this not just in IE11 but in all browsers. (We have at times overridden IE11 hacks with @supports (position: sticky) {}.

And in my testing, this PR appears to not have any adverse effects at all. I did test thoroughly in Chrome on Mac, I haven't yet had an opportunity to test in IE11, my virtual machine is acting up.

So in early review, 👍 👍! But it would be nice if someone else could test in IE11 before this lands. Thanks again.

@ambienthack
Copy link
Contributor Author

ambienthack commented Nov 13, 2020

Made another commit to improve a little bit.
The original code logic (before PR) was to let .interface-interface-skeleton__editor grow ap to 100%:

flex-grow: 1;
max-width: 100%; //doesn't work for nested flexbox in IE

New logic is to set .interface-interface-skeleton__editor width to 100% and let it shrink if needed:

flex: 0 1 100%;

This is more expressive and removes potentially harmful flex-grow: 1;

Shrinking may happen if drawer is visible. BTW what is drawer? Haven't found it in the UI. Only the sidebar & the secondary sidebar.

@jasmussen
Copy link
Contributor

Cool work. Especially with structural CSS, it's good to keep the changes tiny.

BTW what is drawer? Haven't found it in the UI. Only the sidebar & the secondary sidebar.

This is the drawer:

Screenshot 2020-11-13 at 11 31 59

It's a new UI element that only is used in the "Site Editor". The Site Editor menu item becomes available if you activate a block-based theme. There are some here: https://github.com/WordPress/theme-experiments

Copy link
Contributor

@tellthemachines tellthemachines left a comment

Choose a reason for hiding this comment

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

Thanks for spotting and fixing this issue @ambienthack!

Tested on IE11 and both the post and the site editor are looking fine ✅

This should be backported to 5.6 as the issue is reproducible on 5.6 Beta 4 as well.

@tellthemachines tellthemachines added [Type] Bug An existing feature does not function as intended Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Nov 16, 2020
@ambienthack
Copy link
Contributor Author

Thank you guys! Looks like I'm gonna become a contributor with my three lines of code. Time to celebrate 🎉

@tellthemachines tellthemachines merged commit d8d5273 into WordPress:master Nov 17, 2020
@github-actions
Copy link

Congratulations on your first merged pull request, @ambienthack! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts:

https://profiles.wordpress.org/me/profile/edit/

And if you don't have a WordPress.org account, you can create one on this page:

https://login.wordpress.org/register

Kudos!

@github-actions github-actions bot added this to the Gutenberg 9.4 milestone Nov 17, 2020
tellthemachines pushed a commit that referenced this pull request Nov 17, 2020
* Interface: fixed not showing footer in IE11

* Interface: prevent editor from overflowing browser window in IE11

* Interface: better solution to prevent editor from overflowing browser window in IE11
@tellthemachines tellthemachines mentioned this pull request Nov 17, 2020
6 tasks
tellthemachines added a commit that referenced this pull request Nov 17, 2020
* Fix pullquote alignment (#26912)

* Fix/ie11 interface bugs (#26944)

* Interface: fixed not showing footer in IE11

* Interface: prevent editor from overflowing browser window in IE11

* Interface: better solution to prevent editor from overflowing browser window in IE11

Co-authored-by: Daniel Richards <[email protected]>
Co-authored-by: Ambient Hack <[email protected]>
@tellthemachines tellthemachines removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Nov 23, 2020
oxyc referenced this pull request Dec 9, 2020
)

* Set toggle and nav panel as first element in interface skeleton

* Position sidebar toggle alongside header topbar

* Add site and content wrappers to interface skeleton

Previously, the navigation panel and navigation toggle lived within skeleton__body and skeleton__header, respectively. In order to address unexpected keyboard focus order between the nav toggle and nav panel, we create a new element inside of interface-skeleton called skeleton__navigation-sidebar.

To account for the layout changes of introducing a new element, we add two wrapper classes: skeleton__site and skeleton__content. Both ensure that the skeleton__header and skeleton__body shrink their children correctly when the navigation sidebar is open.

* Add and remove padding to ensure document settings title is centered

* Remove toggle from flow of page when sidebar is closed

The navigation sidebar lives alongside the skeleton header and skeleton body. This is to ensure that the children of the body and header shrink when the navigation sidebar is open.

The problem is that when the navigation sidebar is closed, the skeleton body (i.e. the block editor) no longer spans the entire width of the viewport. The sidebar (and toggle) occupy space at the edge of the screen. To address this, we position the navigation toggle absolutely when the sidebar is closed.

* Refactor navigation sidebar directory structure

Because navigation panel and navigation toggle live outside the header and body, we place both components in their own navigation sidebar directory

* Update scss references

* Update navbar-toggle references

* Modify z index of footer

* Remove unnecessary comment

* Remove interface-interface-skeleton__layout

* Use interface-skeleton as container

* Rename skeleton__site to skeleton_editor

* Rename skeleton__navigation sidebar to drawer

* Remove comment

* Use z-index mixin for interface-skeleton__footer

* Use z-index mixin for navigation toggle

* Add missing comma in z-index file

* Update interface drawer label for navigation sidebar

* Recenter document actions

* Create variable for header toolbar min width

* Use navigation sidebar redux actions and selectors in refactored nav sidebar

Navigation sidebar and inserter sidebar state were both moved into the 'core/edit-site' data store. We incorporate any relevant actions and selectors into our source code updates.

* Remove unnecessary isNavigationOpened selector

* Return isNavigationOpened directly from selector

Co-authored-by: Copons <[email protected]>
@sirreal
Copy link
Member

sirreal commented Dec 10, 2020

It looks like this introduced a regression: #27622

Any help finding an IE11 compatible solution would be greatly appreciated 🙇‍♂️

@jasmussen
Copy link
Contributor

jasmussen commented Dec 10, 2020

One thing we can look at as a hotfix of sorts, is to go back to the old CSS but wrapped in a supports. I.e. something like

// new code from this PR

@supports (position: sticky) {
	// old code from before this PR
}

Edit: to expand on what the above does — it's essentially writing IE11 code first, and then overriding that IE11 code with the code for modern browsers. Since IE11 doesn't support position: sticky;, that modern CSS will be ignored.

This is usually the best way to handle the discrepancies between IE11 and modern browsers. Implicit in that is also an acceptance, potentially, of infinitely wide blocks in IE, but perhaps that can serve as a temporary fix until a better one lands?

@ambienthack
Copy link
Contributor Author

ambienthack commented Dec 10, 2020

I think max-width:100% can be added back without a problem for ie11. I removed it in favour of flex: 0 1 100%;

because it seemed enough to limit the width to 100%

@jasmussen
Copy link
Contributor

Cool let's maybe try that. I can test in IE11 on my other machine here. Want to make a PR? Otherwise I can, and feel free to CC me on anything, happy to help.

@ambienthack
Copy link
Contributor Author

Yes, i'll make a PR

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 First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IE11: block editor interface goes beyond window boundaries
5 participants