-
Notifications
You must be signed in to change notification settings - Fork 63
Conversation
Storybook and Tailwind configuration previews: Ready Storybook: https://wordpress.github.io/openverse-frontend/_preview/1880 Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again. You can check the GitHub pages deployment action list to see the current status of the deployments. |
Size Change: +1.64 kB (0%) Total Size: 818 kB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, one small error. The hover state is also meant to be the gray background, and the text darkens a bit:
https://www.figma.com/file/GIIQ4sDbaToCfFQyKMvzr8/Openverse-Design-Library?node-id=2960%3A8723
|
8957c2c
to
277a79e
Compare
@@ -5,7 +5,8 @@ | |||
sizeClasses, | |||
{ | |||
'max-w-[10rem] sm:max-w-[20rem] md:max-w-[16rem]': isHeaderScrolled, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is not related to the desktop searchbar, but it fixes the homepage: when the desktop search type button is focused, it should be white and should not have a border (it has a pink ring instead).
69084f5
to
51d6323
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks great. I thought I'd have more code suggestions but I'm not seeing anything 😆 LGTM!
Fix colors for the homepage content switcher (in the searchbar) Fix colors on hover, add active snapshot This header is only used on the search routes, so there's no need for handling non-search routes Add a Clear button, hide it when the search term is blank Import the VClearButton component
51d6323
to
042ec3e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, matches the mockups!
Would using a flag to differentiate the styles have been easier than splitting the input field into old and new components? That way we wouldn't have to update all the various references to the component.
I feel like it's much easier to safely develop both old and new components when they are separated than to use the flag for styles. It is very easy to accidentally break both :( |
Fixes
Fixes #1879 by @dhruvkb
Description
This PR updates the searchbar colors to match the desktop mockups, and adds the VR tests.
Testing Instructions
The searchbar on the homepage and on the search pages on the desktop widths should match the mockups.
Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin