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

Remove the hover text-decoration transition in InlineLink, Accordion, Timeline and align Prose with the updated InlineLink styles #913

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Remove the hover text-decoration transition in InlineLink, Accordion, Timeline and align Prose with the updated InlineLink styles #913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3bee3ad
we can't animate the text-decoration-thickness, we can animate a bord…
stamat Jan 29, 2025
6c6654d
remove InlineLink animation duration token
stamat Feb 3, 2025
557a466
Remove the InlineLink animation from accordion and timeline modules
stamat Feb 3, 2025
0d4c202
this didn't work, some of these tokens didn't exist - so I'm aligning…
stamat Feb 3, 2025
75990f7
text decoration always inherits the text color, it doesn't need to be…
stamat Feb 3, 2025
71ebeb2
Create old-bears-admire.md
stamat Feb 3, 2025
71bd412
Merge branch 'main' into stamat/text-decoration-cannot-be-animated
stamat Feb 3, 2025
0695721
since I removed base.json I also needed to remove the imports
stamat Feb 3, 2025
9b78f0e
Revert "text decoration always inherits the text color, it doesn't ne…
stamat Feb 3, 2025
fa028d2
add migration path
stamat Feb 3, 2025
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
13 changes: 13 additions & 0 deletions .changeset/old-bears-admire.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
'@primer/brand-primitives': patch
'@primer/react-brand': patch
---

Remove the hover text-decoration transition in `InlineLink`, `Accordion`, `Timeline` and align `Prose` with the updated `InlineLink` styles
Copy link
Collaborator

Choose a reason for hiding this comment

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

Great, thanks @stamat.

On the off-chance that someone was relying on that downstream, let's provide a migration path:

- var(--brand-InlineLink-transition-timing)
+ var(--brand-animation-duration-fast)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Remove the hover text-decoration transition in `InlineLink`, `Accordion`, `Timeline` and align `Prose` with the updated `InlineLink` styles
Removed a redundant hover text-decoration transition in `InlineLink`, `Accordion`, `Timeline` and align `Prose`.
Also removed the following css variable: `--brand-InlineLink-transition-timing`. If you were using this variable, please use `--brand-animation-duration-fast` instead.


In case you were using `var(--brand-InlineLink-transition-timing)` variable please migrate to `var(--brand-animation-duration-fast)`

```diff
- var(--brand-InlineLink-transition-timing)
+ var(--brand-animation-duration-fast)
```
9 changes: 0 additions & 9 deletions packages/design-tokens/src/tokens/functional/components/inline-link/base.json
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion packages/react/src/Accordion/Accordion.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,6 @@ details[open] > .Accordion__summary.Accordion__summary--default .Accordion__summ
text-decoration-color: var(--brand-InlineLink-color-rest);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@stamat i think removing this has caused a visual regression. As you're trying to remove the transition, i'm guessing it's unintentional. Could we restore it please?

text-decoration-thickness: var(--brand-borderWidth-thin);
text-underline-position: under;
transition: var(--brand-InlineLink-transition-timing) text-decoration;
}

.Accordion__content a:hover {
Expand Down
1 change: 0 additions & 1 deletion packages/react/src/InlineLink/InlineLink.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
text-decoration-color: var(--brand-InlineLink-color-rest);
text-decoration-thickness: var(--brand-borderWidth-thin);
text-underline-position: under;
transition: var(--brand-InlineLink-transition-timing) text-decoration;
}

.InlineLink:hover {
Expand Down
1 change: 0 additions & 1 deletion packages/react/src/InlineLink/InlineLink.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import type {BaseProps} from '../component-helpers'
* Design tokens
*/
import '@primer/brand-primitives/lib/design-tokens/css/tokens/functional/components/inline-link/colors-with-modes.css'
import '@primer/brand-primitives/lib/design-tokens/css/tokens/functional/components/inline-link/base.css'

/**
* Main stylesheet (as a CSS Module)
Expand Down
28 changes: 7 additions & 21 deletions packages/react/src/Prose/Prose.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,33 +184,19 @@

.Prose a {
color: var(--brand-InlineLink-color-rest);
text-decoration: none;
position: relative;
text-decoration: underline;
text-decoration-color: var(--brand-InlineLink-color-rest);
text-decoration-thickness: var(--brand-borderWidth-thin);
text-underline-position: under;
}

.Prose a:hover {
text-decoration: none;
text-decoration-thickness: var(--brand-borderWidth-thick);
}

.Prose a::after {
content: '';
border-bottom: var(--brand-borderWidth-thin) solid var(--brand-InlineLink-color-rest);
height: 0;
transition: var(--brand-InlineLink-transition-timing) transform;
transform: scaleY(var(--brand-InlineLink-transition-scale-start));
transform-origin: 50% 100%;
width: 100%;
position: absolute;
bottom: 0;
left: 0;
}

.Prose a:hover::after {
transform: scaleY(var(--brand-InlineLink-transition-scale-end));
}

.Prose a:active::after {
border-color: var(--brand-InlineLink-color-pressed);
.Prose a:active {
color: var(--brand-InlineLink-color-pressed);
}

.Prose code {
Expand Down
1 change: 0 additions & 1 deletion packages/react/src/Prose/Prose.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import type {BaseProps} from '../component-helpers'
import styles from './Prose.module.css'
import '@primer/brand-primitives/lib/design-tokens/css/tokens/functional/components/prose/base.css'
import '@primer/brand-primitives/lib/design-tokens/css/tokens/functional/components/prose/colors-with-modes.css'
import '@primer/brand-primitives/lib/design-tokens/css/tokens/functional/components/inline-link/base.css'

export type ProseProps = {
/**
Expand Down
1 change: 0 additions & 1 deletion packages/react/src/Timeline/Timeline.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@
text-decoration-color: var(--brand-InlineLink-color-rest);
text-decoration-thickness: var(--brand-borderWidth-thin);
text-underline-position: under;
transition: var(--brand-InlineLink-transition-timing) text-decoration;
}

.Timeline__item a:hover {
Expand Down
Loading