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

Try making the hover state of nested blocks more visible #16820

Merged
merged 4 commits into from
Aug 2, 2019

Conversation

kjellr
Copy link
Contributor

@kjellr kjellr commented Jul 30, 2019

Following up from #14961.

Currently, when you hover over the new dashed borders for nested blocks, nothing happens:

old

In the original PR, we briefly discussed improving upon this by adopting a hover state. This PR implements a similar hover state to the one proposed there, but it uses a border-style change instead, which I think is a little more visible than jus the color change. Still could use some iteration though:

updated

Todo:

  • Make this work for an individual column block when its child is selected. Something's blocking that right now.
  • Build in styles for dark mode.

@kjellr kjellr added [Type] Enhancement A suggestion for improvement. Needs Design Feedback Needs general design feedback. [Feature] Nested / Inner Blocks Anything related to the experience of nested/inner blocks inside a larger container, like Group or P labels Jul 30, 2019
@kjellr kjellr self-assigned this Jul 30, 2019
This allows for us to match the existing hover color on hover.
@kjellr
Copy link
Contributor Author

kjellr commented Aug 1, 2019

I've adjusted the colors a little bit to make this a little smoother:

hovers

Now, the all-around hover border color more closely matches the thick left border (and breadcrumb), which feels much more subtle and considered.

In order to make this feel natural, I had to lighten the dashed border colors by a couple steps. This makes them a little less easier to see, but I think it still feels pretty solid overall.

@karmatosed karmatosed self-requested a review August 2, 2019 12:43
Copy link
Member

@karmatosed karmatosed left a comment

Choose a reason for hiding this comment

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

I like this a lot, it's subtle enough to give me feedback but also feels not overwhelming. I do think testing this with some deep level nesting is important but getting it in seems sensible to me as a starting point.

kjellr added 2 commits August 2, 2019 09:19
This ensures that they apply correctly in a 3+ level deep scenario.
@kjellr kjellr marked this pull request as ready for review August 2, 2019 13:51
@kjellr
Copy link
Contributor Author

kjellr commented Aug 2, 2019

Thanks, @karmatosed! I've pushed a couple small fixes:

  • Dark mode styles, to match those already in place for the standard block borders.
  • A fix that allows these hover states to show up in situations where nesting runs 3+ levels deep (like the columns block).

Mind giving this one more look?

@mapk
Copy link
Contributor

mapk commented Aug 2, 2019

After just testing this, I believe it looks good! There's some subtle hover and click targeting that seems off a bit though. I imagine these issues are more about nesting than this specific PR, so I don't want to hold this one back b/c of them. But this is what I was experiencing.

nesting

@kjellr
Copy link
Contributor Author

kjellr commented Aug 2, 2019

I imagine these issues are more about nesting than this specific PR, so I don't want to hold this one back b/c of them. But this is what I was experiencing.

Yeah, I've seen that too. I think the block boundaries need to be fine-tuned a little bit, but that's probably a much more complicated problem. 😄

@karmatosed
Copy link
Member

Works well for me with changes. I stand by we should iterate based on feedback of deeper nesting and I do also agree there is something weird going on about boundaries to fix in another issue.

@karmatosed karmatosed merged commit 87da09e into master Aug 2, 2019
@kjellr kjellr deleted the update/nested-block-hover-state branch August 2, 2019 17:38
@github-actions github-actions bot added this to the Gutenberg 6.3 milestone Aug 2, 2019
gziolo pushed a commit that referenced this pull request Aug 29, 2019
* Try using a solid border for the hover state of nested blocks.

* Try using slightly lighter dashed borders.

This allows for us to match the existing hover color on hover.

* Add dark mode styles.

* Expand hover styles to cover all children of a parent block.

This ensures that they apply correctly in a 3+ level deep scenario.
gziolo pushed a commit that referenced this pull request Aug 29, 2019
* Try using a solid border for the hover state of nested blocks.

* Try using slightly lighter dashed borders.

This allows for us to match the existing hover color on hover.

* Add dark mode styles.

* Expand hover styles to cover all children of a parent block.

This ensures that they apply correctly in a 3+ level deep scenario.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Nested / Inner Blocks Anything related to the experience of nested/inner blocks inside a larger container, like Group or P Needs Design Feedback Needs general design feedback. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants