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

Blockbase: remove unused Button CSS after implementing elements API #6042

Closed

Conversation

MaggieCabrera
Copy link
Contributor

@MaggieCabrera MaggieCabrera commented May 26, 2022

Changes proposed in this Pull Request:

This PR is built on top of #6041

We are removing the CSS that is now covered by the elements API implementation in #6041. The buttons on the search block, file block and button block should look the same as before.

I decided to leave the mixins as they are, in case they are used in other child themes, while removing the includes from Blockbase itself. I'm keeping all the hover stuff so we are not breaking anyone's themes either until that is supported by theme.json too.

@pbking
Copy link
Contributor

pbking commented Jun 7, 2022

This button API doesn't allow us to handle hover states right? So at a minimum we'll need to keep the hover state mixins to keep feature parity.

Also border styles also still seems to still be lacking as something we can define so is that going to have to stay as well?

I think we can be more aggressive that what we've got going on in the PR at the moment though. Typography, color and padding seems to be handled pretty well so I think all of that mixin CSS can go too.

Lastly, I had a little question as to how we might be able to change how we're targeting some non-button stuff. We use some classes like .wp-block-search__button to target search buttons explicitly (and we'll need to continue to for said hover styles at least). But do you think we'll be able to use the new wp-element-button in its stead?

@MaggieCabrera
Copy link
Contributor Author

I think we can be more aggressive that what we've got going on in the PR at the moment though. Typography, color and padding seems to be handled pretty well so I think all of that mixin CSS can go too.

I'm not removing that from the mixin in case the mixin is being used by other elements that don't leverage the elements API, like a 3rd party plugin or block. It's a long stretch to assume, but it's a possibility and keeping the mixin intact doesn't produce extra CSS on Blockbase, only on the child that wants to make use of the mixin.

@MaggieCabrera MaggieCabrera marked this pull request as ready for review June 22, 2022 07:14
@MaggieCabrera
Copy link
Contributor Author

Lastly, I had a little question as to how we might be able to change how we're targeting some non-button stuff. We use some classes like .wp-block-search__button to target search buttons explicitly (and we'll need to continue to for said hover styles at least). But do you think we'll be able to use the new wp-element-button in its stead?

The new class will not be present on blocks that are already present on sites, so I'd rather keep the current markup for the CSS that we have

@MaggieCabrera MaggieCabrera changed the title Try/cleanup buttons css Blockbase: remove unused Button CSS after implementing elements API Jul 4, 2022
@pbking
Copy link
Contributor

pbking commented Jul 6, 2022

Tested and this works as expected (when Gutenberg is installed). Since this requires a version of Gutenberg higher than what is currently in CORE I'm going to tag this item for Blockbase 3.1 (a release for WP 6.1).

@pbking pbking added this to the Blockbase 3.1 milestone Jul 6, 2022
@pbking pbking modified the milestones: Blockbase 3.1, WordPress Core 6.1 Jul 22, 2022
pbking and others added 15 commits August 9, 2022 14:21
* Club: fixing outline button styles

* lint
* Club: fixing line-height of h1 and h2 elements

* Club: fixing size of post and page titles

* Club: post comment form submit button font size update

* Club; pagination links font size

* Club: Remove the underline from query pagination and post navigation blocks

* Club: removing double underline from reply comment link

* Club: changing line-height of the footer elemetns

* Club: same font size for comment meta and content
* fix sass build scripts

* update deployment script to preserve wpcom version suffix
* Club: fluid settings for spacers

* Club: refining spacing

* Club: archive template title  as h1
* Club: removing text-decoration-thickness from theme.json because webkit browsers are not compatible with text-decoration shorthand syntax

* Revert "Club: removing text-decoration-thickness from theme.json because webkit browsers are not compatible with text-decoration shorthand syntax"

This reverts commit 39b0708.

* Club: removing text-decoration-thickness from theme.json because webkit browsers are not compatible with text-decoration shorthand syntax

* Club: adding !important text-decoration-thickness declaration to apply to all the link elements in the page. Otherwise the browser defaults to auto

* Club: fix site title decoration thickness
alaczek and others added 26 commits October 27, 2022 09:31
* Add the load_theme_textdomain function call

* Fix incorrect domain

* Load blockbase translations

* Moved all calls to load_theme_textdomain to be handled by Blockbase (not the children)

* Refactored calls to load_theme_textdomain to not include the default location to simplify the call

* Added load_theme_textdomain call to block canvas

* Removed unrelated videomaker changes

Co-authored-by: Jason Crist <[email protected]>
* Add semi-colon

* Remove blank template

* Tidy CSS formatting

* Move root padding to theme.json

* Add issue links

* Update header markup

* Update post-meta markup

* Update all layout settings

* Update footer credit text

* Remove index.php file

* Update comments markup

* Remove extra word from footer

* Update markup

* Hide avatar

* Remove layout setting from template part

* Remove _unstableLocation from nav
* Add archive template

* Move home to index template

* Indent query markup

* Fix search input text colour

* Fix query part alignment
* move link styles to theme.json

* Remote: Adding the removed text-decoration-thickness property but in theme.json

* Bump min version for Remote to 6.1

Co-authored-by: A8C <[email protected]>
Co-authored-by: Matias Benedetto <[email protected]>
Co-authored-by: Sarah Norris <[email protected]>
)

* Pendant: replacing CSS with theme.json settings for link elements

* Bump min version for Pendant to 6.1

Co-authored-by: Sarah Norris <[email protected]>
* Stewart: Move link color and hover color to theme.json

* move text decoration rules to JSON

* Bump min version for Stewart to 6.1

Co-authored-by: Sarah Norris <[email protected]>
@madhusudhand
Copy link
Member

closing this because it points to an old branch. created pr to trunk here #6714

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.