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

[Test] Archeo link interactivity states #6101

Merged
merged 3 commits into from
Jul 6, 2022

Conversation

getdave
Copy link
Contributor

@getdave getdave commented Jun 22, 2022

Changes proposed in this Pull Request:

This is a complementary PR designed to stress test the proposed changes in WordPress/gutenberg#41786.

That PR enables interactivity states on elements to be defined in theme.json.

This PR tries out that functionality on:

  • top level links.
  • links within the core/navigation block.

Testing

In order to test this PR you will need to be running a copy of Gutenberg with WordPress/gutenberg#41786 checked out and built.

Related issue(s):

WordPress/gutenberg#41786

@madhusudhand
Copy link
Member

@getdave @scruffian

Though it's working as intended, I observe the following un-intended behaviour.
Because the buttons are also links in the HTML, any styles given to links are applied to buttons.

Imagine the button behaviour as follows.

Button Hover state
image image

Now, if I set elements->link:hover style as textDecoration: underline in theme.json, it also gives an underline to all the buttons.
image

Buttons again needs explicit styling to reset it. Curator PR for reference.

@getdave
Copy link
Contributor Author

getdave commented Jun 30, 2022

@madhusudhand I have a fix for that in WordPress/gutenberg#42072

@mikachan
Copy link
Member

mikachan commented Jul 4, 2022

I've removed the color change on hover from theme.json, as I believe this was just for helping to test the GB PR. If there is nothing else to test with this, I think this is ready to bring in.

@getdave getdave marked this pull request as ready for review July 6, 2022 08:25
@getdave getdave force-pushed the try/archeo-link-interactivity-states branch from 3da2422 to 4ea74c6 Compare July 6, 2022 08:29
@getdave
Copy link
Contributor Author

getdave commented Jul 6, 2022

@mikachan I rebased. Is this now good to go?

@mikachan
Copy link
Member

mikachan commented Jul 6, 2022

Yes, LGTM!

@getdave getdave merged commit 8ac496e into trunk Jul 6, 2022
@mikachan mikachan deleted the try/archeo-link-interactivity-states branch July 6, 2022 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Do NOT merge! [Theme] Archeo Automatically generated label for Archeo.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants