-
Notifications
You must be signed in to change notification settings - Fork 2
Card art accessibility improvements #114
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
base: main
Are you sure you want to change the base?
Conversation
@@ -29,7 +29,7 @@ export function makeCard<T extends ScryfallCard>(overrides: Partial<T> = {}): T | |||
scryfall_uri: "<scryfall uri>", | |||
type_line: "<type line>", | |||
set: "<set code>", | |||
image_uris: makeImageUris(), | |||
image_uris: makeImageUris(overrides.name), |
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 so that the image uri does change for the computed property, so the watch on the imageUri
does trigger.
@@ -74,12 +85,14 @@ $default-art-width: 626px; | |||
display: flex; | |||
align-items: center; | |||
justify-content: center; | |||
text-align: center; |
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.
both the style changes here are to make the text and x in error overlay be centered.
:src="imageUri" | ||
:onload="onLoad" | ||
:onerror="onError" | ||
data-testid="card-art-preload" |
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.
needed to add a data-testid here to target it in the test to trigger the load and error events.
src/components/CardArt.vue
Outdated
<div class="shape-x"></div> | ||
<div> | ||
<div class="shape-x"></div> | ||
<p>There was an error loading the card art.</p> |
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.
Thoughts on the wording here?
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 can also occur if we get a server error from the random endpoint, I think.
There was an error loading the next card. You should skip this one.
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.
src/components/CardArt.vue
Outdated
</div> | ||
<div v-else-if="showLoadingOverlay" class="overlay loading"> | ||
<LoadingPulser class="pulser" /> | ||
<p class="vh">Loading next card art...</p> |
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.
Thoughts on the wording here?
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.
Loading next card
(no dots)
@@ -74,12 +85,15 @@ $default-art-width: 626px; | |||
display: flex; | |||
align-items: center; | |||
justify-content: center; | |||
text-align: center; | |||
padding: 1rem; |
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.
added some padding here because the class/saga art crops don't give you much space and the text was butting up against the edge.
Uh oh!
There was an error while loading. Please reload this page.