Fix product card accessibility #589
Conversation
|
@tauthomas01, this is looking good. Thoughts on using |
|
Hi @svinkle, thank you for the review. I added the focus state on the whole card now |
|
@tauthomas01 Curious does this apply to all "cards" throughout? I also called out the blog card in #452. |
|
@svinkle Good question, this change applies on
For the blog card, I will handle in a separate PR. |
|
@tauthomas01 Any timeline on merging this? I need to have this component tested with Fable on a separate issue, but would prefer these changes to be merged and available during testing. |
|
@svinkle I'll need some devs to review this, I'll get back to you shortly. |
tauthomas01
force-pushed
the
fix-product-card-accessibility
branch
from
648d981
to
55c5a1b
Compare
| {%- else -%} | ||
| <div class="card__content"> | ||
| <h2 class="card__text h3"> | ||
| <a href="{{ product_card_product.url | default: '#' }}" class="full-unstyled-link"> |
There was a problem hiding this comment.
Another thing here, I wonder if the price part should be part of the link as well 🤔 ?
video
There was a problem hiding this comment.
I saw on other product cards with image that when you have prices and vendor, it's also highlighting so I made the changes: https://screenshot.click/16-34-8qevw-au1v7.mp4
tauthomas01
dismissed stale reviews from svinkle and ludoboludo
via
e4ad151
| @@ -42,6 +42,10 @@ | |||
| justify-content: center; | |||
| } | |||
|
|
|||
| .card--text-only.card--product { | |||
| position: static; | |||
There was a problem hiding this comment.
A side effect, which I think is nice, is that on mobile when you click on the card it now only highlights the product title rather that the whole product card.
|
@svinkle , I made a small change (#589 (comment)) Whenever you're ready to approve, I can merge this |
|
@tauthomas01 Sorry, I'm not sure what this refers to. What was changed? |
On product card with no image, when you hover the price, the link is clickable. Previously, when you hover the price, we were seeing the hover state, but the link was not clickable. |
|
@tauthomas01 Gotcha. Looks good! 👍 |
* Fix accessibility for product cards * remove default heading space * Fix collage * add focus on card hover * fixed no media case + spacing on collage * include links to the whole product card with no media
Why are these changes introduced?
Fixes #450
The goal of this PR is to fix the accessibility issues on product card component.
Best way to code review is to hide whitespaces.
What approach did you take?
visually-hiddenproduct titlearia-hiddenfor badgesThis affect Collage section (product) and product card component (featured collection section, collection page)
Other considerations
Demo links
Checklist