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 polish #6138

Merged
merged 4 commits into from
Apr 12, 2018
Merged

Add polish #6138

merged 4 commits into from
Apr 12, 2018

Conversation

jasmussen
Copy link
Contributor

This PR contains 3 sets of fixes.

The first polishes the link dialog a bit, by using color variables. Notably it fixes a regression with the Image link:

screen shot 2018-04-12 at 11 01 05

There's still an issue here, though, as soon as you start typing in the link dialog for images, it disappears. I suspect isEditing is kicking in and hiding the block toolbars. CC: @noisysocks

The second part unifies the styles between readmore and pagination:

screen shot 2018-04-12 at 11 38 52

The third is a fix to make sure that placeholders fit in tight spots, by including a shorter Media Library label. It also makes the font sizes on those buttons consistent with all other Gutenberg buttons:

screen shot 2018-04-12 at 12 03 17

@jasmussen jasmussen added the [Type] Enhancement A suggestion for improvement. label Apr 12, 2018
@jasmussen jasmussen self-assigned this Apr 12, 2018
@jasmussen jasmussen requested review from karmatosed, swissspidy and a team April 12, 2018 10:07
@mtias
Copy link
Member

mtias commented Apr 12, 2018

The third is a fix to make sure that placeholders fit in tight spots, by including a shorter Media Library label.

Love this.

@@ -129,7 +129,7 @@ export const settings = {
value={ id }
render={ ( { open } ) => (
<Button isLarge onClick={ open }>
{ __( 'Add from Media Library' ) }
{ __( 'Media Library' ) }
Copy link
Member

Choose a reason for hiding this comment

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

Are these duplicated buttons needed if that's the default for the component?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure — I just changed the labels already there.

@jasmussen
Copy link
Contributor Author

Pushed a fix to tweak the breadcrumb arrow to use the same as the image url button. I can see that I'll need to rebase in a second.

screen shot 2018-04-12 at 12 20 41

Joen Asmussen added 4 commits April 12, 2018 12:23
This uses variables for colors and shadows, and also fixes a regression with the link dialog when linking an image.

However there's still a bug here, where as soon as you start typing in the image link dialog, the entire thing disappears and you can't type. This is possibly due to isEditing mode being invoked there. Can you take a look, @noisysocks @karmatosed?
This adds a white background, and some things, to the Pagination block. This will only be visible if a user loads a stylesheet into the editor.

Fixes #1467 (comment), cc @swissspidy.
This makes the placeholders fit better in small breakpoints.
This makes the left arrow consistent with the one used when creating links on images.
@jasmussen jasmussen force-pushed the polish/more-polish branch from 59f3d76 to 6425897 Compare April 12, 2018 10:24
@mtias
Copy link
Member

mtias commented Apr 12, 2018

Looks good to me. 🚢

@jasmussen
Copy link
Contributor Author

Woop woop!

@jasmussen jasmussen merged commit a481c2c into master Apr 12, 2018
@jasmussen jasmussen deleted the polish/more-polish branch April 12, 2018 12:07
@noisysocks
Copy link
Member

I suspect isEditing is kicking in and hiding the block toolbars. CC: @noisysocks

It looks like this regression was introduced in #5513. I'll create an issue.

@jasmussen jasmussen added this to the 2.7 milestone Apr 13, 2018
@aduth
Copy link
Member

aduth commented Apr 18, 2018

I suspect isEditing is kicking in and hiding the block toolbars. CC: @noisysocks

It looks like this regression was introduced in #5513. I'll create an issue.

Related: #5942, #5894

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants