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

Search block styles in the style variations, option 1 #525

Open
wants to merge 9 commits into
base: trunk
Choose a base branch
from

Conversation

carolinan
Copy link
Contributor

@carolinan carolinan commented Oct 9, 2024

Description

This PR changes the border radius of the search block button and search input field.
It also sets the border color of the search input field to accent color 6, the color with the opacity.
In one variation, it also updates the font family for the button.

With this option, the input field styles are limited to this single block, and managed in multiple theme.json files.

Screenshots
Noon, after

Search block styles in the Noon variation

Dusk, after:
Search block styles in the Dusk variation

Testing Instructions

Add a search form.
Confirm that the style matches the expectations listed in this issue: #485
Repeat the test for all style variations

Copy link

github-actions bot commented Oct 9, 2024

Preview changes

You can preview these changes by following the link below:

I will update this comment with the latest preview links as you push more changes to this PR.
⚠️ Note: The preview sites are created using WordPress Playground. You can add content, edit settings, and test the themes as you would on a real site, but please note that changes are not saved between sessions.

@carolinan carolinan marked this pull request as ready for review October 9, 2024 05:41
Copy link

github-actions bot commented Oct 9, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: carolinan <[email protected]>
Co-authored-by: juanfra <[email protected]>
Co-authored-by: beafialho <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@beafialho
Copy link
Contributor

I'm having issues with my testing setup and I don't see the same as your screenshot 😞 However, if that's how it looks for you, it's looking as intended.

@carolinan
Copy link
Contributor Author

carolinan commented Oct 9, 2024

Yes the shadow for example is working well for me on the search block button.

But I am concerned about how well the border radius on the input works with other inputs, mainly the comment form which always has a border radius of 4px.

@carolinan
Copy link
Contributor Author

Like in this screenshot
inputs

@carolinan carolinan requested a review from juanfra October 9, 2024 10:39
@beafialho
Copy link
Contributor

the comment form which always has a border radius of 4px.

Visually, the comment form radius should inherit the styles of each variation.

Also, if we could remove this option for the search block, it would be good. It looks less than ideal in every variation.

Captura de ecrã 2024-10-09, às 11 41 44

@carolinan
Copy link
Contributor Author

There is no way for the theme to remove this option, that I am aware of 🤔 Not even with PHP or JavaScript.

@carolinan
Copy link
Contributor Author

Visually, the comment form radius should inherit the styles of each variation.

I agree with this. Short term I can try to add it to this PR.

I am still thinking of how we could do this for all inputs including the third party blocks as requested here:
#484. Yes, I opened this issue, but because it was requested by a user on Twitter.
In case you missed it, especially the select list looks bad in my opinion.

@beafialho
Copy link
Contributor

Yes, I saw that. It would be great if we could do it for all inputs.

In case you missed it, especially the select list looks bad in my opinion.

I have replied with a visual suggestion for the select list.

@carolinan carolinan mentioned this pull request Oct 9, 2024
@carolinan carolinan added the [Type] Enhancement A suggestion for improvement. label Oct 9, 2024
@carolinan
Copy link
Contributor Author

If we handle the comment form and the select list in separate PR's, this is ready for review.

Copy link
Member

@juanfra juanfra left a comment

Choose a reason for hiding this comment

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

It looks well. I noticed that when you switch the style variation to one of these and then go back to one where the input has a rounded shape, then that doesn't take effect. For example, trying to go back to the default style.

Screen.Recording.2024-10-14.at.16.21.15.mov

@carolinan
Copy link
Contributor Author

How strange.
It looks like the variations that are supposed to have 0 border radius on the inputs still have 4px.

My guess would be that it is the styles> blocks > block name > CSS that is not clearing / refreshing.

@carolinan
Copy link
Contributor Author

Because of this bug, I don't think it is worth implementing this change. I think it might be too confusing that the style changes depending on what order you preview the theme variations in...

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.

3 participants