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 editor toolbars and their sizes #891

Merged
merged 7 commits into from
May 30, 2017
Merged

Conversation

jasmussen
Copy link
Contributor

@jasmussen jasmussen commented May 24, 2017

This is a bundle PR, and improves a few things.

  1. It normalizes toolbars back to the design they have in the mockups. Through only my own fault, they had regressed to grow in size (46px), and we'd forgotten they were supposed to be smaller (38px)
  2. It maximizes the hit area of the buttons, by including the margin between buttons.
  3. Fixes an issue where the inline toolbar could be subject to lineheight bleed.
  4. Renames $blue-medium to $blue-medium-500, to match the hierarchy of the other medium blues

Screenshots:

screen shot 2017-05-24 at 15 02 02

screen shot 2017-05-24 at 14 54 04

screen shot 2017-05-24 at 14 54 08

@jasmussen jasmussen self-assigned this May 24, 2017
}
}

// compensate for making the button hit area include the visual spacing
.components-toolbar__control.components-button + .components-toolbar__control.components-button {
margin-left: -3px;
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this make it asymmetric?

Copy link
Member

Choose a reason for hiding this comment

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

When the rule is not here, there's as much space for the button left as for the button right. With this rule, the entire spacing goes to the right button.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, so this one is a bit hard to explain, to be honest, but it seems to be visually working for me, so if you're seeing any visible weirdness do let me know.

The thing is — the toolbar background, the wrapping container, is now 38px tall including the borders.

Inside this, are 36x36px buttons. So that means they take up the FULL vertical space.

Now these 36x36px buttons have 3px padding. This adds that 3px white bit of space around the dark gray toggled version. This was previously margin.

The thing is, if you put two of these 36x36px buttons next to each other, the white space between them will be 6px. By adding a negative 3px margin to element + element we essentially ensure that that spacing between button is always 3px, when there's more than one.

@ellatrix
Copy link
Member

Looking good!

Not sure if this is a result of the smaller buttons, but the number on the heading buttons seems slightly off:

screenshot 2017-05-24 20 25 46

@jasmussen
Copy link
Contributor Author

Not sure if this is a result of the smaller buttons, but the number on the heading buttons seems slightly off:

Yeah I'm increasingly thinking it might be worth exploring an alternate way of picking heading size, as discussed in #522. So I'm thinking about how to best approach that.

In the mean time, I polished a bit:

screen shot 2017-05-25 at 11 46 32

@mtias
Copy link
Member

mtias commented May 29, 2017

Please, more specific pull request titles 😁

@jasmussen
Copy link
Contributor Author

Please, more specific pull request titles 😁

😂

My apologies. I will do better.

@jasmussen jasmussen changed the title Miscellaneous polish Polish the editor toolbars and their sizes May 30, 2017
jasmussen added 7 commits May 30, 2017 10:26
Better done early on, right? This relabels the medium blue color with a number, same as the grays.
Previously it did not use the editor monospace font, colors, or sizes.
Biggish change:

- The toolbar has grown through regressions, and ended being 46px tall. It's only supposed to be 38px. This fixes that.
- Increased the clickable aread to include padding. The bigger the clickable area, the better. As such, hover and active style are now set on the child SVG element, so as to retain the look of the padding.
This adds negative left margins to successive editor bar buttons, compensating for their increased hit area, so it visually still looks like there's a 3px margin.
@jasmussen
Copy link
Contributor Author

Unless anyone objects, I think I'll merge this in the next couple of hours.

@jasmussen jasmussen merged commit ace6128 into master May 30, 2017
@jasmussen jasmussen deleted the add/misc-polish branch May 30, 2017 09:38
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