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 z-index issues #22416 #22757

Closed
wants to merge 1 commit into from
Closed

fix z-index issues #22416 #22757

wants to merge 1 commit into from

Conversation

abder
Copy link
Contributor

@abder abder commented May 29, 2020

Description

This commit fixes the issue described in #22416

How has this been tested?

Manually on the browser

Screenshots

This commit fixes the following 2 issues
1- The block toolbar is appearing on top of the notice

Screen Shot 2020-05-29 at 6 30 33 PM

2- on scrolling enough, the block toolbar also appears on top of the top bar

Screen Shot 2020-05-29 at 1 02 41 PM

Types of changes

Changed the z-index value for both the notice and the top bar

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@ZebulanStanphill
Copy link
Member

This causes the (recently added) parent-selector button to be unusable when the block toolbar is right below the top toolbar on scroll.
image

#21056 specifically changed the z-indexes so the block toolbar would appear above the top bar. Whether that was a good idea or not is debatable.

So we need to figure out how to make the parent-selector button usable when the block toolbar goes sticky while still keeping the block toolbar at a lower elevation than the top toolbar.

The only solution I can think of is moving the parent-selector button. I know we only just added it, but I'm starting to have second thoughts about its placement.

Perhaps if #22673 is merged, we could move the parent-selector to the position that the movers currently occupy, and have them float to the left of the block toolbar, rather than above. However, one of the reasons for moving the movers in the first place is to make the block toolbar positioning logic simpler. Moving the parent-selector button there would undo that. (It also breaks the sort of vague "going up the hierarchy" metaphor that the current position of the parent-selector button conveys, though I'm not sure most people would even notice that design detail.) But at least in the short-term, it seems like the best option that doesn't drastically alter the presentation of the button.

Maybe the parent-selector button would make more sense as an item in the "More options" ellipsis menu. On the other hand, that makes it a bit more cumbersome to use, as well as less discoverable. But that does seem like the least-weird place to put it.

Anyone else got any ideas?

@ZebulanStanphill ZebulanStanphill added General Interface Parts of the UI which don't fall neatly under other labels. [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release and removed [Type] Bug An existing feature does not function as intended labels May 29, 2020
@abder
Copy link
Contributor Author

abder commented May 29, 2020

@ZebulanStanphill I didn't notice that button until now.
if #22673 is merged, i think your idea of moving that button to the previous position of the movers button would be great despite the fact that the new position won't have that context of "going up hierarchy" as you said. looks good with the sticky position too.

captured (1)

Base automatically changed from master to trunk March 1, 2021 15:43
@skorasaurus
Copy link
Member

Thanks for making this Pull Request and the effort and time you spent on it.

This issue at #22416 appears to have been resolved; so I'm closing the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
General Interface Parts of the UI which don't fall neatly under other labels. [Type] Regression Related to a regression in the latest release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants