Add the component for the homepage grid #2060
Conversation
dhruvkb
added
🟨 priority: medium
|
Storybook and Tailwind configuration previews: Ready Storybook: https://wordpress.github.io/openverse-frontend/_preview/2060 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: +12.4 kB (+1%) Total Size: 873 kB
ℹ️ View Unchanged
|
|
The gallery looks awesome ✨ a few notes: Focus styleThe square ring looks odd wrapping a circle element.
Based on this comment on focus ring styles, it would be great to apply the
Alt textStill pending the alt text for all images. Added to my tasks. Vertical alignmentNot sure if this comment belong to this PR, yet sharing it to not miss it. The vertical center alignment of search area and gallery is not even. I think
Broken linkI found just one: Universe Image-06. Despite the above, it looks awesome. Great work. |
Co-authored-by: Olga Bulat <[email protected]>
Delay in focus when jumpingWhen jumping between gallery images in dev using the keyboard, the focus loads its style with a small delay, showing the pink ring first and the white ring later. Here is a screen recording.
Not sure if this comment belongs here but adding it as I also shared feedback on how focus ring styles work. All the rest looks excellent ✨ Non-blocking ideaLayout in
|
|
@panchovm I did not experience the behaviour seen by you. On my monitor, with width of 2560px, this is the appearance of the gallery. There is white space but the gallery is centered in the right field.
|
|
Oh, I see. Nonetheless, in both cases, the side and in-between spaces look uneven. Here is a wireframe of the idea.
It is an idea that should not block the work made, so please feel free to avoid it. We can tackle it in the future. |
|
@panchovm the diagram you show should be from 2xl breakpoint and above? |
|
Yes! Sorry for not being clear @dhruvkb |
|
Can we merge this PR as is, and open a separate issue for the 2xl fixes, @panchovm , @dhruvkb ? It would be nice to unblock other homepage development. |
|
I'm more than happy to get this merged and fix the 2xl later. |
I think we can take it as the second approval :) Considering that these changes are only active when the new_header is on, I believe it's safe to merge after the test failures are fixed. |
|
Sure. No problem merging it. |
dhruvkb
force-pushed
the
homepage_grid
branch
from
f4667b6 to
0841d05
Compare
| @@ -35,6 +48,15 @@ for (const dir of languageDirections) { | |||
| }) | |||
| ) | |||
|
|
|||
| breakpoints.describeEvery(({ expectSnapshot }) => | |||
| test(`${dir} full page redesigned`, async ({ page }) => { | |||
There was a problem hiding this comment.
If we add something like "old design" to the other test, it will be easier to clean up after the full transition to the new designs.
There was a problem hiding this comment.
Makes sense, updated test names.
Fixes
Related to #1433 by @panchovm.
Description
This PR adds the gallery grid component shown on the redesigned home page. Care has been taken to minimise disruption to the existing homepage.
Testing Instructions
VHomeGallery.new_headerflag enabled (http://localhost:8443/?ff_new_header=on).Checklist
Update index.md).main) ora parent feature branch.
errors.
Developer Certificate of Origin
Developer Certificate of Origin