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

Add focus style to the Inserter search and improve alignments. #1393

Merged
merged 2 commits into from
Jun 28, 2017

Conversation

afercia
Copy link
Contributor

@afercia afercia commented Jun 23, 2017

This PR adds a focus style to the inserter search field. It uses the default WordPress style (border + box-shadow) and introduces two new colors in _variables.scss:

$blue-focus: #5b9dd9;
$shadow-focus: 0px 0px 2px rgba( #1e8cbe, .8 );

It also tries to improve the left alignments of the menu titles, icons, and search field. See the screenshot below, before and after:

screen shot 2017-06-23 at 12 58 11

Note 1: the left alignment can't be pixel-perfect because of the different SVG icons shapes.

Note 2: looks like the search field needs a z-index property otherwise the top border is not visible in some cases when scrolling the menu:

screen shot 2017-06-23 at 12 32 25

Note 3: in Safari, the search field is a bit taller than in other browsers. Not sure if we want to fix this:

screen shot 2017-06-23 at 12 38 19

Fixes #1378

@jasmussen
Copy link
Contributor

Nice, thank you. We're looking at augmenting the design of the inserter, including moving the search box up top, so some of the little niggles we should probably address separately. See also #1325 (comment).

I'm a bit weary of introducing new colors. As tracked in #573, I'd rather we embrace Hugo's colors for everything editor, and keep all those new colors collected in a single place so if the core ticket referenced becomes a blocker, we can replace them all in one fell swoop. That would include the focus styles, so they would be the same.

As such, I'd prefer we address the focus styles as part of #573, and instead use the focus style defined here:

https://github.com/WordPress/gutenberg/blob/master/components/icon-button/style.scss#L30

... but perhaps we could optimize that, so as to make it easier to replace all focus styles in one fell swoop should we need to. For example a variable such as this:

$focus-style: 0 0 0 1px $blue-medium-400, 0 0 2px 1px $blue-medium-400;

What do you think?

@afercia
Copy link
Contributor Author

afercia commented Jun 28, 2017

Yep I've also seen @mtias proposal to experiment switching the inserter to a single column so this PR could be a bit outdated.

One option could be just making some elements inherit the focus styles from WordPress. Maybe some default styles should be provided by the platform Gutenberg is plugged in. Looking for example at the Excerpt textarea, it just inherits the focus style from WP.
The default, subtle, box-shadow](https://core.trac.wordpress.org/browser/trunk/src/wp-admin/css/forms.css?rev=40823#L9) WordPress sets on input fields could be a bit annoying but it is also possible to consider to remove it from the WP rule.
All the other focus styles, including outline: none, are already provided by WP.
What do you think?

@jasmussen
Copy link
Contributor

I do think we need a separate push to address focus styles in a larger way. #904 still isn't fully addressed, and the gist is that if we can find a way to make sure the focus style doesn't "linger" on click (i.e. mimic native button focus styles), this suggests one design, and if we can't do that, it suggests a different design. In both cases it feels like the style discussion should be addressed there.

@afercia
Copy link
Contributor Author

afercia commented Jun 28, 2017

Do feel free to close this PR 🙂

@jasmussen
Copy link
Contributor

I pushed a quick change to the styles, so we can fix them in one place as part of #904. I think the other changes are valuable, so going to merge when the tests complete.

@jasmussen jasmussen merged commit d6b29f6 into master Jun 28, 2017
@jasmussen jasmussen deleted the update/inserter-search-focus-style-alignment branch June 28, 2017 09:31
youknowriad pushed a commit that referenced this pull request Jan 17, 2020
Merge master (v1.13.1) back to develop
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.

Inserter search focus style and alignments
2 participants