Cards: Use svg instead of text inside the placeholder image #1886
Conversation
✅ Deploy Preview for boosted ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
site/layouts/shortcodes/card.html
Outdated
| @@ -22,9 +22,11 @@ | |||
| {{- $rtl := .Get "rtl" | default false -}} | |||
|
|
|||
| <div class="card{{ if eq $borders false }} border-0{{ end }}{{ if and $borders (eq $background "dark") }} border-dark{{ end }}"> | |||
| <svg class="bd-placeholder-img card-img-top" width="100%" height="162" xmlns="http://www.w3.org/2000/svg" role="img" aria-label="{{ if $rtl }}تعليق على الصورة{{ else }}Image cap{{ end }}" preserveAspectRatio="xMidYMid slice" focusable="false"> | |||
| <svg class="bd-placeholder-img card-img-top" width="100%" height="169" xmlns="http://www.w3.org/2000/svg" role="img" aria-label="{{ if $rtl }}تعليق على الصورة{{ else }}Image cap{{ end }}" preserveAspectRatio="xMidYMid slice" focusable="false"> | |||
There was a problem hiding this comment.
Currently wondering if Image cap is a good value for an aria-label attribute?
It's outside the frame of the current PR so we may discuss it in a distinct issue.
There was a problem hiding this comment.
Seems to be on Bootstrap Side so yes, let's tackle it there directly!
| @@ -20,7 +20,7 @@ Below is an example of a basic card with mixed content and a fixed width. Cards | |||
|
|
|||
| {{< example >}} | |||
| <div class="card" style="width: 18rem;"> | |||
| {{< placeholder width="100%" height="180" class="card-img-top" text="Image cap" >}} | |||
| {{< placeholder width="100%" height="180" class="card-img-top" text="icon" >}} | |||
There was a problem hiding this comment.
Was the transition from 'Image cap' to 'icon' validated by our a11y expert or do we need to ask him for a review?
Shouldn't it be tackled on Bootstrap's side? 🤔
There was a problem hiding this comment.
Added the right label for upcoming a11y review and yeah it might be proposed on their side as well.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
as discuss with @louismaximepiton, there's some improvement that has to be made in another PR, so ok on this one |
| <rect width="100%" height="100%" fill="{{ (index $.Site.Data.grays 3).hex }}"/> | ||
| <text x="50%" y="50%" fill="{{ (index $.Site.Data.grays 5).hex }}" dy=".3em">{{ if $rtl }}تعليق على الصورة{{ else }}Image cap{{ end }}</text> | ||
| <svg x="30%" y="30%" width="40%" height="40%" viewBox="0 0 24 24" fill="#00000014" aria-hidden="true" focusable="false"> |
There was a problem hiding this comment.
We don't have a corresponding value in our $.Site.Data.grays list?
There was a problem hiding this comment.
No, I don't think so. I chose to have a kind of watermark with opacity since we have some use cases using colored backgrounds and the grey was pretty ugly to use with. Tell me what you think the best between:
There was a problem hiding this comment.
None 😄 Should probably be a dark gray but nevermind, it's not that related to the PR. Edge case.
|
Kudos, SonarCloud Quality Gate passed!
|
Note: Please transform
- [ ]into- (NA)in the description when things are not applicableRelated issues
Related to #1677.
Description
Use a svg instead of text.
Motivation & Context
Better a11y + might be better detected inside
pa11y-ci.Closer to design.
Types of change
Live previews
Checklist
Contribution
Accessibility
Design
Development
Documentation
Checklist (for Core Team only)
After the merge