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

Polish the block switcher styles #492

Merged
merged 3 commits into from
Apr 24, 2017
Merged

Polish the block switcher styles #492

merged 3 commits into from
Apr 24, 2017

Conversation

jasmussen
Copy link
Contributor

This PR adds a little polish to the block switcher.

  • It normalizes padding, which fixes an alignment issue
  • It adjusts hover colors
  • It removes the arrow from the dropdown — I'm still chewing on this one but let's try out out for size

One thing that needs an extra pair of eyes is that I added a negative z index on .blocks-editable. It seems a recent change to that component added a position: relative; inline style. Which is fine, but it also caused the switcher to .. underlap, is that a word? ... the contents of the block, regardless of how high a z index the switcher had. By giving the editable a negative z index this fixes that. But I can't quite predict the consequences.

Your eyes, please, @aduth and others. A recent change added `position: relative;` as an inline style on `blocks-editable`. This caused the block switcher to be underneath the editable, regardless of its z-index. By lowering the z-index of the editable, this fixes it.
- Remove arrow, let's see how this feels.
- Adjust hover colors.
@jasmussen jasmussen self-assigned this Apr 24, 2017
@youknowriad
Copy link
Contributor

Should the hover style of the block switcher be similar to the other controls?

@jasmussen
Copy link
Contributor Author

Should the hover style of the block switcher be similar to the other controls?

Maybe :D — I should have clarified that this is another thing I'm trying on for size and may revert on.

This PR at least reduces the hover styles to two:

  1. action buttons, which have a blue hover style
  2. toggles, which have a bordered outline hover style with no text color change

My reasoning for style #2 is that this is what WordPress does at the moment for toggle buttons like bold, italic etc. We might find that this isn't solid, and in that case I'd like to try a unified singular hover style for all things. Which is likely to be different from both the two above, btw.

But for now, I'm hoping we can try this and get a feel for it.

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

In #460, I felt that the style #2 is not ideal. Too many black background (especially when we have multiple inline formats applied), I tend to prefer the design we had in the prototypes.

By giving the editable a negative z index this fixes that. But I can't quite predict the consequences.

I don't know the consequences either. It could have an impact on the "floating blocks" maybe, but fine to address when we see a problem.

Anyway this is good to merge as is. 🚢

@jasmussen jasmussen merged commit 9511797 into master Apr 24, 2017
@jasmussen jasmussen deleted the update/switcher-styles branch April 24, 2017 13:01
@jasmussen
Copy link
Contributor Author

Thank you Riad!

I agree the dark style can be a bit heavy. But it's one of the styles that passes the accessibility guidelines. Let's keep our ideas flowing but move forward, a little bit at a time :)

@ellatrix
Copy link
Member

ellatrix commented Apr 24, 2017

@jasmussen Hm, adding the z-index made the editable fields unfocusable. Any reason this was added?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants