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

Block list appender cleanup: Add gray background and dark theme styles, fix hover state. #14619

Closed
wants to merge 1 commit into from

Conversation

kjellr
Copy link
Contributor

@kjellr kjellr commented Mar 25, 2019

This PR cleans up the block list appender:

  • It adds a gray background to align with the styles in #14241 and #12367.
  • It adds proper styles for themes that declare support for dark-editor-style.
  • It corrects a bug where the hover state was being overridden by default button styles.

The easiest way to test is to use the block manager: Set the "Paragraph" block to be hidden. Once you do that, the block list appender should appear:

front-end


Screenshots

Before:

Screen Shot 2019-03-25 at 9 01 15 AM

Before (Hover):

Screen Shot 2019-03-25 at 9 01 07 AM

After:

Screen Shot 2019-03-25 at 9 16 47 AM

After (Hover):

Screen Shot 2019-03-25 at 9 16 53 AM

After (Dark Theme):

Screen Shot 2019-03-25 at 9 17 55 AM

After (Dark Theme, Hover):

Screen Shot 2019-03-25 at 9 18 26 AM

@kjellr kjellr added the General Interface Parts of the UI which don't fall neatly under other labels. label Mar 25, 2019
@kjellr kjellr self-assigned this Mar 25, 2019
@kjellr kjellr requested a review from jasmussen March 25, 2019 13:20
@kjellr kjellr requested a review from chrisvanpatten as a code owner March 25, 2019 13:20
@kjellr kjellr added the Needs Design Feedback Needs general design feedback. label Mar 25, 2019
@chrisvanpatten
Copy link
Contributor

Looking at the gif, why is the alternate appender the full width of the canvas? Has it always done that?

@chrisvanpatten
Copy link
Contributor

(FWIW the styles look super good; just weird that the width is different!)

@kjellr
Copy link
Contributor Author

kjellr commented Mar 25, 2019

Looking at the gif, why is the alternate appender the full width of the canvas? Has it always done that?

It is weird! Looking at the history, it looks like this appender previously showed up inside of a block wrapper. That meant it got all the styles of a .block-editor-block-list__block and .wp-block. In my GIF example, it doesn't have any sort of max width or margins, so it just takes up all the available space.

I'm not 100% sure the best way to solve that (changing it purely via CSS will likely mean it won't adhere to theme styles for the content width).

@kjellr
Copy link
Contributor Author

kjellr commented Mar 25, 2019

Just noticed that #14241 basically does this same thing. Should hold off on this one until that lands.

@jasmussen
Copy link
Contributor

Nice PR, agreed on holding off until 14241. But great trick on testing this new appender.

Agree with Chris, though, this appender should be no wider than the blocks it inserts. Let's consider exploring that either in this PR, or a separate one, how we can restore that.

@kjellr
Copy link
Contributor Author

kjellr commented Apr 23, 2019

Closing, since #14241 was merged.

@kjellr kjellr closed this Apr 23, 2019
@youknowriad youknowriad deleted the update/block-list-appender-styles branch May 27, 2020 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
General Interface Parts of the UI which don't fall neatly under other labels. Needs Design Feedback Needs general design feedback.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants