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/image overlaps control #15966

Closed
wants to merge 1 commit into from

Conversation

senadir
Copy link
Contributor

@senadir senadir commented Jun 3, 2019

closes issue #15923

Description

increase block editor contextual toolbar z index so that floated images don't overlap it

How has this been tested?

see #15923 for a way to reintroduce and then click on image to test

Screenshots

image

Types of changes

bug fix

Checklist:

  • [x ] My code is tested.
  • [x ] My code follows the WordPress code style.
  • [x ] My code follows the accessibility standards.
  • [x ] My code has proper inline documentation.
  • [x ] I've included developer documentation if appropriate.

@gziolo gziolo added First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository Needs Design Feedback Needs general design feedback. Good First Review A PR that's suitable for someone looking to contribute for the first time by reviewing code [Feature] Blocks Overall functionality of blocks labels Jun 3, 2019
@jasmussen
Copy link
Contributor

Hi @senadir, thank you for the pull request!

This branch:

this branch

Master:

master

This is very nice, and it doesn't seem like the z-index change has any negative side effects.

However I see a bunch of changes that seem to be related to the Date Picker component. Can you explain what those changes intend to accomplish?

In general, it is good practice to keep pull requests as specific as possible and fix only one issue at a time. This makes reviews both easier and faster.

Thanks again.

@senadir
Copy link
Contributor Author

senadir commented Jun 4, 2019

That's a a really amature mistake from my side, those commits come from another pull request #15962
I will delete them from this pull request so we can merge #511f21d

@jasmussen
Copy link
Contributor

No worries. I still see a lot of calendar stuff in this PR, it's probably easiest to just edit that out of this PR rather than trying to revert individual commits. But once that's gone, we can put this in a milestone to review! Thanks again.

@senadir senadir force-pushed the fix/image-overlaps-control branch from e946658 to 82e90bb Compare June 8, 2019 19:04
@senadir
Copy link
Contributor Author

senadir commented Jun 11, 2019

@jasmussen the unrelated commits have been fixed so you can review again to merge

@senadir
Copy link
Contributor Author

senadir commented Jun 12, 2019

@jasmussen after checking the conflict files it seems you already fixed this issue in #15537 and merged it 6 days ago. so I will close this PR since it's not needed

@senadir senadir closed this Jun 12, 2019
@senadir senadir deleted the fix/image-overlaps-control branch June 12, 2019 09:09
@jasmussen
Copy link
Contributor

Oh interesting, I must have unknowingly fixed it, so I very much appreciate you verifying that it fixes this issue.

And thanks so much for the PR, really do appreciate it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository Good First Review A PR that's suitable for someone looking to contribute for the first time by reviewing code Needs Design Feedback Needs general design feedback.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants