Skip to content
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.

Sign up for GitHub

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

Feat / a11y improve the contrast of non-texts #55

Merged
merged 2 commits into from
Jan 30, 2024
Merged

Feat / a11y improve the contrast of non-texts #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
77ac755
feat(a11y): add solid header background color to ensure accessibility
MelissaDTH Jan 25, 2024
fb7337f
refactor(a11y): enhance text readability
MelissaDTH Jan 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/ui-react/src/components/Card/Card.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
line-height: 36px;
white-space: nowrap;
text-overflow: ellipsis;
text-shadow: 0 2px 4px rgba(0, 0, 0, 0.14), 0 4px 5px rgba(0, 0, 0, 0.12), 0 1px 10px rgba(0, 0, 0, 0.2);
text-shadow: 0 2px 4px rgba(0, 0, 0, 0.75);

@include responsive.mobile-only {
font-size: 24px;
Expand All @@ -54,7 +54,7 @@
.meta {
justify-content: space-between;
padding: 16px;
background: linear-gradient(to top, rgba(0, 0, 0, 0.7), rgba(0, 0, 0, 0));
background: linear-gradient(to top, rgba(0, 0, 0, 0.8), rgba(0, 0, 0, 0));
}

&:hover {
Expand Down
4 changes: 2 additions & 2 deletions packages/ui-react/src/components/VideoDetails/VideoDetails.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
color: var(--primary-color);
font-family: var(--body-font-family);
font-weight: 400;
text-shadow: 0 2px 4px rgba(0, 0, 0, 0.14), 0 3px 4px rgba(0, 0, 0, 0.12), 0 1px 5px rgba(0, 0, 0, 0.2);
text-shadow: 0 2px 4px rgba(0, 0, 0, 0.75);
}

.mainPadding {
Expand Down Expand Up @@ -37,7 +37,7 @@

.info {
position: relative;
width: 70%;
width: 50%;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should remove @include responsive.tablet-only() { width: 350px; } couple of lines further. Because it makes the text description really narrow.

Screenshot 2024-01-29 at 12 51 06

So you agree @royschut ? If so, can you make perform this change.

Copy link

@royschut royschut Jan 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't that break the purpose again, making text appear above the image? @langemike

Copy link

@langemike langemike Jan 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is true. I think due to the length of the text it gives a really unproportionate feeling. But with a small text it feels more balanced. I have to agree with you. Example with small text:
Screenshot 2024-01-30 at 09 31 33

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should actually show less of the image and more of the text. But that's a design (UI) choice.

max-width: 650px;
min-height: 440px;

Expand Down
3 changes: 3 additions & 0 deletions packages/ui-react/src/utils/theming.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,8 @@ export const setThemingVariables = (config: Config) => {
if (root && headerBackground) {
root.style.setProperty('--header-background', headerBackground);
root.style.setProperty('--header-contrast-color', calculateContrastColor(headerBackground));
} else {
root.style.setProperty('--header-background', 'rgba(0, 0, 0, 0.85)');
root.style.setProperty('--header-contrast-color', '#ffff');
}
};
Loading