-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat(styles): tokenize the Link component styles #3590
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: f9aab11 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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.
Good job 👍
Do not hesitate to contact me if one of my comments is not clear
); | ||
} | ||
|
||
@include utilities.high-contrast-mode() { |
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.
The link is missing a hover and focus state on high contrast mode, you can check w3 website to know how they handle this: https://www.w3.org/WAI/ARIA/apg/practices/names-and-descriptions/
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.
There is also a mixin to do this in the design-system:
// example
@include high-contrast-mode() {
color: LinkText;
&:hover {
color: HighlightText;
}
}
```
Here you can find he available color names: https://www.w3.org/TR/css-color-4/#css-system-colors
import { MetaComponent } from '@root/types'; | ||
|
||
const meta: MetaComponent = { | ||
id: 'link-component', |
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.
We usually use random UUID here, you can generate one on this website for axample: https://www.uuidgenerator.net/
Use links to navigate users to another location, such as a different site, resource, or section within the same page. | ||
</div> | ||
|
||
Apply the override classes for custom styling as needed. |
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.
Not sure I get this sentence, what do you mean?
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.
When you create a new snapshot file you also need to create the matching visual regression test here: https://github.com/swisspost/design-system/tree/main/packages/documentation/cypress/snapshots/components
- The
*.snapshot.stories.ts
file creates this page: https://preview-3590--swisspost-design-system-next.netlify.app/iframe.html?id=snapshots--link&viewMode=story - The
cypress/*.snapshot.ts
files uses this page to take snapshots that are compared every time we release changes
<div> | ||
${['bg-white', 'bg-dark'].map( | ||
bg => html` | ||
<div class="${bg} d-flex flex-column gap-regular p-regular mt-regular"> |
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.
Here you also have to set a data-color-mode="light"
or data-color-mode="dark"
attribute so that the correct set of token is used.
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.
Please relocate this component so it appears under "Foundations/Typography/Link"!
And also move the files.
); | ||
} | ||
|
||
@include utilities.high-contrast-mode() { |
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.
There is also a mixin to do this in the design-system:
// example
@include high-contrast-mode() {
color: LinkText;
&:hover {
color: HighlightText;
}
}
```
Here you can find he available color names: https://www.w3.org/TR/css-color-4/#css-system-colors
Co-authored-by: Oliver Schürch <[email protected]>
…st/design-system into 3424-component-text-link
# Conflicts: # packages/styles/src/elements/_index.scss
…st/design-system into 3424-component-text-link
Quality Gate passedIssues Measures |
No description provided.