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.

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: Support CSS transitions in RAC #7488

Merged
merged 10 commits into from
Jan 14, 2025
Merged

feat: Support CSS transitions in RAC #7488

merged 10 commits into from
Jan 14, 2025

Conversation

devongovett
Copy link
Member

Fixes #6225, closes #5627

For discussion. This adds support for CSS transitions to RAC for entry and exit animations, in addition to the existing support for keyframe animations. Transitions have the benefit that they are interruptible. I noticed in the ActionBar PR that if you clicked fast enough you could see it "jump". That's because when a keyframe animation is interrupted, it jumps to the end, and then the next animation starts. With CSS transitions, the next animation continues from where the previous one left off instead, avoiding this problem.

Currently there is no API change needed for this:

data-entering – waits for keyframe animations to complete, but does not wait for css transitions. For CSS transitions, the starting styles can be applied here, and after one frame, they are removed so that the transition to the default style occurs.
data-exiting – now waits for css transitions as well.

I'm not sure if this is confusing or not, or whether we should introduce another data attribute instead.

isEntering: 250,
transition: '[opacity, translate]',
transitionDuration: {
default: 250,
Copy link
Member Author

@devongovett devongovett Dec 6, 2024

Choose a reason for hiding this comment

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

A bit weird that this is not symmetric with isExiting. isEntering won't work because it is only applied for a single frame. With CSS transitions, isEntering is more like "starting style".

@rspbot
Copy link

rspbot commented Dec 6, 2024

@rspbot
Copy link

rspbot commented Dec 17, 2024

Copy link
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

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

I think I like sticking with is entering and is exiting, that's pretty common language around this.

That it waits now for the transitions seems fine on the surface. I'm having trouble coming up with cases where people wouldn't want this behavior.

@devongovett
Copy link
Member Author

I think I like sticking with is entering and is exiting, that's pretty common language around this.

The potential confusion is that it does not wait for transitions in the entering state. The entering state acts as the starting state of the transition, then it is removed causing the transition to the default state. So while the transition state is happening (I would call that entering), isEntering is actually false.

@snowystinger
Copy link
Member

Ah, I see. I got hit by the confusion.
The mental model I was thinking of was https://reactcommunity.org/react-transition-group/css-transition

It doesn't look like we lose the ability to have asymmetrical animations? so I'm ok with it, I think we'll just need to document it much like CSSTransition did and explain what the base/to states are.

@devongovett devongovett marked this pull request as ready for review January 10, 2025 17:05
@@ -485,7 +485,6 @@ function Nav({currentPageName, pages}) {
<details key={section.title} open={section.isActive}>
<summary style={{fontWeight: 'bold'}}>
<ChevronRight size="S" /> {section.title}
{section.title === 'Components' && <VersionBadge version={Object.values(section.pages)[0][0].preRelease} style={{marginLeft: 'auto', fontWeight: 'normal'}} />}
Copy link
Member Author

Choose a reason for hiding this comment

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

Since we added Autocomplete (the first component alphabetically) all of RAC was getting marked as alpha again 😂

@rspbot
Copy link

rspbot commented Jan 10, 2025

LFDanLu
LFDanLu previously approved these changes Jan 10, 2025
Copy link
Member

@LFDanLu LFDanLu left a comment

Choose a reason for hiding this comment

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

Verified the behavior of the transitions in ActionBar/Modal/Popover/Tooltip, definitely much smoother than before when spam opening/closing. Just one small suggestion but otherwise happy for this to go out as is for people to try and give feedback

@@ -424,16 +411,13 @@ The `className` and `style` props also accept functions which receive states for
Popovers also support entry and exit animations via states exposed as data attributes and render props. `Popover` will automatically wait for any exit animations to complete before it is removed from the DOM.
Copy link
Member

Choose a reason for hiding this comment

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

I think the explanation in the CSS transition page styling is good, it might be worthwhile linking to it from the Popover/Tooltip/Modal/etc pages and/or via the JSDOC of the isEntering/isExiting props so it is more discoverable.

Copy link
Member

@reidbarber reidbarber left a comment

Choose a reason for hiding this comment

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

Change looks good, minor docs suggestions.


### CSS animations

Overlay components such as [Popover](Popover.html) and [Modal](Modal.html) support entry and exit animations via the `[data-entering]` and `[data-exiting]` states, or via the corresponding render prop functions. You can use these states to apply CSS keyframe animations. These components will automatically wait for any exit animations to complete before they are removed from the DOM.
For more complicated animations, you can also apply CSS keyframe animations using the same `[data-entering]` and `[data-exiting]` states.
Copy link
Member

@reidbarber reidbarber Jan 10, 2025

Choose a reason for hiding this comment

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

maybe better:

Suggested change
For more complicated animations, you can also apply CSS keyframe animations using the same `[data-entering]` and `[data-exiting]` states.
For more complex animations, you can also apply CSS keyframe animations using the same `[data-entering]` and `[data-exiting]` states.

@@ -268,6 +308,8 @@ Overlay components such as [Popover](Popover.html) and [Modal](Modal.html) suppo
}
```

Note that unlike CSS transitions, keyframe animations are not interruptable. If the user opens and closes an overlay quickly, the animation may appear to jump to the ending state before the next animation starts.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Note that unlike CSS transitions, keyframe animations are not interruptable. If the user opens and closes an overlay quickly, the animation may appear to jump to the ending state before the next animation starts.
Note that unlike CSS transitions, keyframe animations are not interruptible. If the user opens and closes an overlay quickly, the animation may appear to jump to the ending state before the next animation starts.

@rspbot
Copy link

rspbot commented Jan 10, 2025

@rspbot
Copy link

rspbot commented Jan 10, 2025

## API Changes

react-aria-components

/react-aria-components:AutocompleteProps

-AutocompleteProps {
-  children: ReactNode
-  defaultInputValue?: string
-  filter?: (string, string) => boolean
-  inputValue?: string
-  onInputChange?: (string) => void
-  slot?: string | null
-}

@react-spectrum/s2

/@react-spectrum/s2:Tabs

 Tabs {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   children?: ReactNode
   defaultSelectedKey?: Key
   density?: 'compact' | 'regular' = 'regular'
   disabledKeys?: Iterable<Key>
   id?: string
   isDisabled?: boolean
+  isIconOnly?: boolean
   keyboardActivation?: 'automatic' | 'manual' = 'automatic'
-  labelBehavior?: 'show' | 'hide' = 'show'
   onSelectionChange?: (Key) => void
   orientation?: Orientation = 'horizontal'
   selectedKey?: Key | null
   slot?: string | null
 }

/@react-spectrum/s2:TabsProps

 TabsProps {
   UNSAFE_className?: string
   UNSAFE_style?: CSSProperties
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   children?: ReactNode
   defaultSelectedKey?: Key
   density?: 'compact' | 'regular' = 'regular'
   disabledKeys?: Iterable<Key>
   id?: string
   isDisabled?: boolean
+  isIconOnly?: boolean
   keyboardActivation?: 'automatic' | 'manual' = 'automatic'
-  labelBehavior?: 'show' | 'hide' = 'show'
   onSelectionChange?: (Key) => void
   orientation?: Orientation = 'horizontal'
   selectedKey?: Key | null
   slot?: string | null
 }

Copy link
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

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

there's now a width animation introduced by this PR, it's pretty minor though

@devongovett
Copy link
Member Author

there's now a width animation introduced by this PR, it's pretty minor though

which component?

@devongovett devongovett added this pull request to the merge queue Jan 13, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 13, 2025
@snowystinger
Copy link
Member

which component?

Sorry, was verifying from testing session. It's the ActionBar for Table

@devongovett devongovett added this pull request to the merge queue Jan 14, 2025
Merged via the queue into main with commit 1b425ca Jan 14, 2025
30 checks passed
@devongovett devongovett deleted the rac-css-transition branch January 14, 2025 01:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants