8000 fix(buttons): border transition update by andrew-ronaldson · Pull Request #7096 · patternfly/patternfly · GitHub
[go: up one dir, main page]

Skip to content

fix(buttons): border transition update#7096

Merged
mcoker merged 3 commits intopatternfly:mainfrom
andrew-ronaldson:button-border-fix
Sep 24, 2024
Merged

fix(buttons): border transition update#7096
mcoker merged 3 commits intopatternfly:mainfrom
andrew-ronaldson:button-border-fix

Conversation

@andrew-ronaldson
Copy link
Collaborator
@andrew-ronaldson andrew-ronaldson commented Sep 21, 2024

fixes #7036

@patternfly-build
Copy link
Collaborator
patternfly-build commented Sep 21, 2024

@andrew-ronaldson andrew-ronaldson marked this pull request as ready for review September 23, 2024 13:17
Copy link
Member
@srambach srambach left a comment

Choose a reason for hiding this comment

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

Spectacular! ✨

// Secondary
--#{$button}--m-secondary--Color: var(--pf-t--global--text--color--brand--default);
--#{$button}--m-secondary--BorderColor: var(--pf-t--global--border--color--brand--default);
--#{$button}--m-secondary--TransitionDuration: var(--pf-t--global--motion--duration--fade--short);
Copy link
Collaborator
@mattnolting mattnolting Sep 24, 2024

Choose a reason for hiding this comment

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

Nice 👍. Instead of reassigning css vars, I suggest using a base transition duration for buttons, like:

...
--#{$button}--TransitionDuration: var(--pf-t--global--motion--duration--fade--short);
}

.button {
  transition-duration: var(--#{$button}--TransitionDuration);

We're working to pivot away from reassigning css variables as this approach affects performance implications and using prop: val; definitions are more readable in devtools.

Copy link
Contributor

Choose a reason for hiding this comment

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

We're working to pivot away from reassigning css variables as this approach affects performance implications and using prop: val; definitions are more readable in devtools.

We can talk about this as a team and see when/if it makes sense to do that. It's always something we can do as a non-breaking follow-up once we have a strategy for it 👍

Copy link
Contributor
@mcoker mcoker left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@mcoker mcoker merged commit 93f55bb into patternfly:main Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Buttons: Fix transition on secondary and tertiary buttons

5 participants

0