Skip to content
This repository has been archived by the owner on Jan 26, 2022. It is now read-only.

Try adding simple search + file button styles #235

Merged
merged 2 commits into from
Nov 15, 2021

Conversation

kjellr
Copy link
Collaborator

@kjellr kjellr commented Nov 15, 2021

Alternate to #202.

This takes a purely CSS approach to styling the file and search block buttons.

Something like this is required until the following Gutenberg issues are resolved:

Test data:
https://gist.github.com/jffng/263436360988105ab73c35effd494fe0

Before After
Screen Shot 2021-11-15 at 12 21 21 PM Screen Shot 2021-11-15 at 12 20 33 PM

@kjellr kjellr added the [Type] Enhancement New feature or request label Nov 15, 2021
@kjellr kjellr requested a review from jffng November 15, 2021 17:23
@kjellr kjellr self-assigned this Nov 15, 2021
@@ -25,7 +25,7 @@ function twentytwentytwo_support() {
add_theme_support( 'wp-block-styles' );

// Enqueue editor styles.
add_editor_style( get_template_directory_uri() . '/style.css' );
add_editor_style( 'style.css' );
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This undoes the commit here: #220 (comment)

In my testing, this prevented the stylesheet from loading in the editor.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I can't recreate this but I'm fine with the change, based on the docs for this function.

text-decoration-thickness: 1px;
text-underline-offset: 0.25ch;
text-decoration-thickness: 1px;
text-underline-offset: 0.25ch;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changing spaces to tabs. 😉

border: none;
color: var(--wp--preset--color--background);
font-size: var(--wp--preset--typography--font-size--normal);
padding: calc(.667em + 2px) calc(1.333em + 2px);
Copy link
Collaborator Author

@kjellr kjellr Nov 15, 2021

Choose a reason for hiding this comment

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

We could leave this padding line out to keep this even more minimal — it makes this mirror the normal button padding, but I'm a little indifferent about it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think let's keep it.

Copy link
Collaborator

@jffng jffng left a comment

Choose a reason for hiding this comment

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

Some minor comments but LGTM.

style.css Outdated Show resolved Hide resolved
@@ -25,7 +25,7 @@ function twentytwentytwo_support() {
add_theme_support( 'wp-block-styles' );

// Enqueue editor styles.
add_editor_style( get_template_directory_uri() . '/style.css' );
add_editor_style( 'style.css' );
Copy link
Collaborator

Choose a reason for hiding this comment

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

I can't recreate this but I'm fine with the change, based on the docs for this function.

border: none;
color: var(--wp--preset--color--background);
font-size: var(--wp--preset--typography--font-size--normal);
padding: calc(.667em + 2px) calc(1.333em + 2px);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think let's keep it.

background-color: var(--wp--preset--color--primary);
border-radius: 0;
border: none;
color: var(--wp--preset--color--background);
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you think about adding this line:

Suggested change
color: var(--wp--preset--color--background);
color: var(--wp--preset--color--background);
cursor: pointer;

Copy link
Collaborator Author

@kjellr kjellr Nov 15, 2021

Choose a reason for hiding this comment

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

I recall there being a major discussion around this cursor behavior at some point in Core. I think we should leave it to be the default behavior for right now — if that gets changed it should happen in Gutenberg.

Co-authored-by: Jeff Ong <[email protected]>
@kjellr kjellr merged commit a11fb49 into trunk Nov 15, 2021
@kjellr kjellr deleted the try/simple-search-file-button-styles branch November 15, 2021 18:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
[Type] Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants