feat(alert): add dismiss transition, timing and duration props - #1683
feat(alert): add dismiss transition, timing and duration props#1683NokNok-0 wants to merge 1 commit into
Conversation
|
|
@NokNok-0 is attempting to deploy a commit to the Bergside Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe Alert component now supports configurable dismissal transitions, delayed ChangesAlert dismiss transition
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant DismissButton
participant Alert
participant onDismissCallback
DismissButton->>Alert: click dismiss
Alert->>Alert: apply transition classes
Alert->>Alert: wait for duration
Alert->>onDismissCallback: invoke onDismiss
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Add 'transition', 'timing', and 'duration' props to the Alert component so the dismiss animation can be customized, mirroring vanilla Flowbite's Dismiss class options. - transition (default: transition-opacity) — Tailwind transition utility class - duration (default: 300) — length of the transition in ms - timing (default: ease-out) — Tailwind timing-function utility class onDismiss now fires after the transition has finished (or immediately when duration is 0). The transition classes are also exposed via the optional alert.transition theme key for global overrides. Closes themesberg#1092
2b7fd9c to
437ead4
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ui/src/components/Alert/Alert.tsx`:
- Around line 99-105: Update handleDismissClick in Alert to defer dismissal only
when duration > 0, so duration={0} invokes onDismiss synchronously even when the
default transition is present. Add or update the zero-duration test with a
synchronous assertion immediately after the click rather than relying on
awaiting user.click.
- Around line 121-123: Update the Alert component’s transition handling to
derive one effective numeric duration from the themed transition value,
including overrides such as duration-700, and reuse that value for both the
rendered transition class and the onDismiss timeout. Replace the independent
prop-default timer usage while preserving existing callback behavior.
- Around line 93-95: Update the dismiss flow in Alert’s handleDismissClick and
timeout callback to retain the original React mouse event and pass that same
event to onDismiss when the timer fires, removing the fabricated MouseEvent
while preserving the existing duration behavior.
- Around line 117-120: Update the transition class construction in the Alert
component’s twMerge call to use Tailwind’s arbitrary duration syntax, such as
duration-[${duration}ms] or the existing alert-duration CSS variable, so custom
durations are applied correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 615cdef3-0d7f-4ce0-a052-3c3e7cdb635c
📒 Files selected for processing (5)
apps/web/content/docs/components/alert.mdxapps/web/examples/alert/alert.transition.tsxapps/web/examples/alert/index.tspackages/ui/src/components/Alert/Alert.test.tsxpackages/ui/src/components/Alert/Alert.tsx
| const timeoutId = setTimeout(() => { | ||
| onDismiss?.(new MouseEvent("click") as unknown as React.MouseEvent<HTMLButtonElement>); | ||
| }, duration); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the Alert component around the reported lines.
if git ls-files 'packages/ui/src/components/Alert/Alert.tsx' | grep -q .; then
echo "== file exists =="
wc -l packages/ui/src/components/Alert/Alert.tsx
echo "== relevant lines =="
cat -n packages/ui/src/components/Alert/Alert.tsx | sed -n '1,180p'
else
echo "Alert.tsx not found"
fd -i 'Alert\.tsx$' .
fi
echo "== search onDismiss usages =="
rg -n "onDismiss|new MouseEvent|onDismiss\?\(" packages/ui/src || true
echo "== inspect package/tool config maybe relevant =="
fd -i "package.json|tailwind.config" . | sed -n '1,80p'Repository: themesberg/flowbite-react
Length of output: 8677
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== inspect related test around onDismiss assertions =="
cat -n packages/ui/src/components/Alert/Alert.test.tsx | sed -n '120,225p'
echo "== TypeScript check around mock/expect behavior without compiling repo code? =="
echo "node version:"
node --version || true
node - <<'JS'
class SyntheticEvent {
constructor(type, target, currentTarget) {
this.type = type;
this.target = target;
this.currentTarget = currentTarget;
}
}
const native = new MouseEvent("click");
const cast = native;
console.log(JSON.stringify({
type: cast.type,
target: cast.target,
currentTarget: cast.currentTarget,
stopPropagationType: typeof cast.stopPropagation,
}, null, 2));
console.log(JSON.stringify({
syn: new SyntheticEvent("click", "button", "root").target,
eventTarget: cast.target,
}, null, 2));
JSRepository: themesberg/flowbite-react
Length of output: 4767
Pass the original dismiss click event to the deferred onDismiss callback.
The timed callback currently receives a fabricated native MouseEvent, so callers won’t get a React synthetic click event with the correct target/currentTarget. Keep the event from handleDismissClick and pass it through when the timeout runs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ui/src/components/Alert/Alert.tsx` around lines 93 - 95, Update the
dismiss flow in Alert’s handleDismissClick and timeout callback to retain the
original React mouse event and pass that same event to onDismiss when the timer
fires, removing the fabricated MouseEvent while preserving the existing duration
behavior.
| const handleDismissClick: ComponentProps<"button">["onClick"] = (event) => { | ||
| // When a transition is configured, play it before firing `onDismiss`. | ||
| if (transition || duration > 0) { | ||
| setIsDismissing(true); | ||
| return; | ||
| } | ||
| onDismiss?.(event); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make duration={0} synchronous.
transition defaults to "transition-opacity", so this condition is true even when duration is 0. The callback is therefore deferred through setTimeout(..., 0), contrary to the documented immediate behavior. The test's await user.click(...) masks this ordering bug.
Use duration > 0 as the deferral condition, and add a synchronous assertion for the zero-duration case.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ui/src/components/Alert/Alert.tsx` around lines 99 - 105, Update
handleDismissClick in Alert to defer dismissal only when duration > 0, so
duration={0} invokes onDismiss synchronously even when the default transition is
present. Add or update the zero-duration test with a synchronous assertion
immediately after the click rather than relying on awaiting user.click.
| twMerge( | ||
| transition, | ||
| `duration-${duration}`, | ||
| timing, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)(Alert\.tsx|tailwind|postcss|package\.json|\.lint|eslint|stylelint|.*\.config\.(js|ts|mjs|cjs))$|Alert' || true
echo "== target snippet =="
if [ -f packages/ui/src/components/Alert/Alert.tsx ]; then
nl -ba packages/ui/src/components/Alert/Alert.tsx | sed -n '1,180p'
fi
echo "== duration/static Tailwind config candidates =="
rg -n "duration-|transitionDuration| safelist|content|preset|plugins|tailwindcss" -S . -g '!node_modules' -g '!dist' -g '!build' -g '!coverage' | head -200
echo "== lint configs mentioning duration =="
rg -n "duration|transition|valid|invalid" -S . -g '*lnt*' -g '!node_modules' -g '!dist' -g '!build' | head -200 || trueRepository: themesberg/flowbite-react
Length of output: 1043
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Alert.tsx relevant lines =="
awk '{printf "%6d\t%s\n", NR, $0}' packages/ui/src/components/Alert/Alert.tsx | sed -n '105,130p'
echo "== Alert.tsx duration prop/types =="
awk '{printf "%6d\t%s\n", NR, $0}' packages/ui/src/components/Alert/Alert.tsx | sed -n '1,120p'
echo "== tailwind config duration/content =="
awk '{printf "%6d\t%s\n", NR, $0}' packages/ui/tailwind.config.cjs
echo "---"
awk '{printf "%6d\t%s\n", NR, $0}' apps/web/tailwind.config.cjs | sed -n '1,260p'
echo "== runtime duration values in Alert.tsx/usages =="
rg -n "duration|transitionDuration|twMerge\\(|duration-" packages/ui/src/components/Alert packages/ui/tailwind.config.cjs -S
echo "== lint rules mentioning invalid transition/duration =="
rg -n "transitionDuration|duration-|valid|Invalid|invalid|CSS variable|arbitrary" -S . -g '!node_modules' | head -200Repository: themesberg/flowbite-react
Length of output: 40429
🌐 Web query:
Tailwind CSS arbitrary values duration- arbitrary value transition duration
💡 Result:
To apply an arbitrary transition duration in Tailwind CSS, use the square bracket syntax directly within the class name [1][2]. For example:
If you are using a CSS variable, you can use the same syntax [1]: Alternatively, Tailwind provides a shorthand syntax for CSS variables using parentheses, which automatically wraps your value in the var function [1]: This functionality allows you to generate one-off transition durations on the fly without needing to extend your theme configuration [2][3]. As with all arbitrary values, this syntax integrates seamlessly with responsive modifiers (e.g., md:duration-[500ms]) and interactive states (e.g., hover:duration-[1s]) [1][4][3].Citations:
- 1: https://tailwindcss.com/docs/transition-duration
- 2: https://v3.tailwindcss.com/docs/transition-duration
- 3: https://agk.io/posts/arbitrary-values-variants-in-tailwindcss/
- 4: https://v3.tailwindcss.com/docs/adding-custom-styles
Use arbitrary transition duration values.
duration-${duration} generates classes like duration-300, which Tailwind does not treat as arbitrary values and won’t include for non-default/custom durations. Use duration-[${duration}ms] or duration-[var(--alert-duration)] instead.
🧰 Tools
🪛 GitHub Check: 🕵 Lint
[warning] 117-117:
Classname 'duration-' is not a Tailwind CSS class!
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ui/src/components/Alert/Alert.tsx` around lines 117 - 120, Update
the transition class construction in the Alert component’s twMerge call to use
Tailwind’s arbitrary duration syntax, such as duration-[${duration}ms] or the
existing alert-duration CSS variable, so custom durations are applied correctly.
Source: Linters/SAST tools
| theme.transition?.base, | ||
| theme.transition?.duration, | ||
| theme.transition?.timing, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Keep the callback delay aligned with themed duration.
A theme can override the rendered class to duration-700, but the timeout still uses the prop default of 300. onDismiss can unmount the alert before its themed transition finishes. Model the effective duration as one shared numeric value for both CSS and the timer.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ui/src/components/Alert/Alert.tsx` around lines 121 - 123, Update
the Alert component’s transition handling to derive one effective numeric
duration from the themed transition value, including overrides such as
duration-700, and reuse that value for both the rendered transition class and
the onDismiss timeout. Replace the independent prop-default timer usage while
preserving existing callback behavior.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ui/src/components/Alert/Alert.tsx`:
- Around line 73-81: Update the Alert dismissal flow around the useEffect so the
timeout deadline is captured once when dismissal begins and is not reset when
onDismiss changes during the transition. Store or otherwise stabilize the
clicked dismissal callback/deadline, keep the effect from restarting on parent
rerenders caused by inline callbacks, and add a regression test covering
rerenders during the CSS transition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 88c5f5f4-a063-44b0-a530-dff116d6ebb9
📒 Files selected for processing (5)
apps/web/content/docs/components/alert.mdxapps/web/examples/alert/alert.transition.tsxapps/web/examples/alert/index.tspackages/ui/src/components/Alert/Alert.test.tsxpackages/ui/src/components/Alert/Alert.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/web/examples/alert/index.ts
- apps/web/content/docs/components/alert.mdx
- packages/ui/src/components/Alert/Alert.test.tsx
- apps/web/examples/alert/alert.transition.tsx
| useEffect(() => { | ||
| if (!isDismissing) { | ||
| return; | ||
| } | ||
| const timeoutId = setTimeout(() => { | ||
| onDismiss?.(new MouseEvent("click") as unknown as React.MouseEvent<HTMLButtonElement>); | ||
| }, duration); | ||
| return () => clearTimeout(timeoutId); | ||
| }, [isDismissing, duration, onDismiss]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate Alert files =="
fd -a 'Alert\.(tsx|ts|jsx|js)$' . | sed 's#^\./##'
echo "== target file outline/size =="
wc -l packages/ui/src/components/Alert/Alert.tsx 2>/dev/null || true
ast-grep outline packages/ui/src/components/Alert/Alert.tsx 2>/dev/null || true
echo "== target file relevant contents =="
cat -n packages/ui/src/components/Alert/Alert.tsx | sed -n '1,140p'
echo "== usages of Alert =="
rg -n "<Alert|Alert " packages -g '*.{ts,tsx,js,jsx}' | head -120Repository: themesberg/flowbite-react
Length of output: 8034
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Alert tests relevant contents =="
cat -n packages/ui/src/components/Alert/Alert.test.tsx | sed -n '1,260p'
echo "== package config for jsdom timers/react =="
for f in package.json packages/ui/package.json; do
if [ -f "$f" ]; then
echo "-- $f --"
node -e "const p=require('./$f'); console.log(JSON.stringify({scripts:p.scripts,dependencies:p.dependencies,devDependencies:p.devDependencies}, null, 2))" | sed -n '1,220p'
fi
done
echo "== focused searches for tests around dismissing/transition/inline callbacks =="
rg -n "onDismiss|dismiss|transition|duration|setInterval|setTimeout|re-render|rerender|render\(\\\(" packages/ui/src/components/Alert packages/ui/src/components -g '!**/*.js' -g '!**/*.map' | sed -n '1,240p'Repository: themesberg/flowbite-react
Length of output: 13891
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== vitest config =="
fd -a '^(vitest|vite|rollup|rslib)\.(config|configuration)\.(ts|js|mjs|cjs)$' packages/ui | sort | while read -r f; do
echo "-- $f --"
cat -n "$f" | sed -n '1,220p'
done
echo "== focused Alert searches =="
rg -n "onDismiss|dismiss|transition|duration|setInterval|setTimeout|re-render|rerender|render\\(" packages/ui/src/components/Alert packages/ui/src/components -g '!**/*.js' -g '!**/*.map' | sed -n '1,260p'
echo "== React dependency versions =="
node - <<'JS'
const fs = require('fs');
for (const f of ['package.json','packages/ui/package.json']) {
if (!fs.existsSync(f)) continue;
const p = JSON.parse(fs.readFileSync(f,'utf8'));
const deps = {...(p.dependencies||{}), ...(p.devDependencies||{})};
console.log(f, deps.react, deps['react-dom']);
}
JSRepository: themesberg/flowbite-react
Length of output: 31536
🌐 Web query:
React useEffect cleanup timeout rerender clearTimeout setTimeout dependency changes behavior
💡 Result:
In React, the useEffect cleanup function plays a critical role in managing side effects that require stopping, cancelling, or undoing operations [1][2]. When using setTimeout (or similar async operations like subscriptions or intervals) within useEffect, the cleanup function is essential to ensure that pending operations from a previous render do not interfere with subsequent ones [2][3]. React guarantees the following behavior regarding cleanup execution [2][4]: 1. Timing of Cleanup: React always executes the cleanup function from the previous render before running the effect function for the next render [2][3]. If a component unmounts, the cleanup function is executed one final time [1][2]. 2. Dependency Changes: If you provide a dependency array, React only re-synchronizes the effect (running the cleanup, then the new effect) when one of the values in that array changes [5][4]. If the dependency array is empty ([]), the effect runs only once after the initial mount, and the cleanup runs only when the component unmounts [2][4]. 3. Interaction with setTimeout: When you use setTimeout within an effect, returning clearTimeout(timeoutId) in the cleanup function is the standard pattern to prevent the timeout callback from executing if the component updates or unmounts before the timer expires [2]. Because React cleans up the previous render's effect before starting the next one, this mechanism ensures that at most one timeout is active for that effect at any given time [2]. If you find that your effect is running too often—such as on every render—it is typically because the dependency array is missing or includes values that change on every render [5][6]. You can avoid this by either ensuring the dependencies are stable or by using functional state updates (e.g., setCount(c => c + 1)) to remove the state variable from the effect's dependency list entirely [7][8]. In summary, the cleanup function does not run on every render by default; it only runs when the dependencies change or the component unmounts [2][6]. By cleaning up the previous timeout before establishing a new one, you maintain predictable behavior and avoid race conditions or memory leaks [2][3].
Citations:
- 1: https://react.dev/reference/react/useEffect
- 2: https://react.dev/learn/synchronizing-with-effects
- 3: https://reacttraining.com/blog/useEffect-cleanup
- 4: https://legacy.reactjs.org/docs/hooks-effect.html
- 5: https://react.dev/learn/lifecycle-of-reactive-effects
- 6: https://stackoverflow.com/questions/57023074/why-is-the-cleanup-function-from-useeffect-called-on-every-render
- 7: https://react.dev/learn/removing-effect-dependencies
- 8: https://overreacted.io/a-complete-guide-to-useeffect/
Keep the dismissal deadline fixed at click time.
The dismiss effect depends on onDismiss, so an inline dismissal callback causes the timeout to be cleared and restarted on every parent rerender during the CSS transition; a constant callback/deadline should be captured when clicked, with a rerender-during-transition regression test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ui/src/components/Alert/Alert.tsx` around lines 73 - 81, Update the
Alert dismissal flow around the useEffect so the timeout deadline is captured
once when dismissal begins and is not reset when onDismiss changes during the
transition. Store or otherwise stabilize the clicked dismissal
callback/deadline, keep the effect from restarting on parent rerenders caused by
inline callbacks, and add a regression test covering rerenders during the CSS
transition.
1 participant
Summary
Adds
transition,timing, anddurationprops to the<Alert>component so the dismiss animation can be customized, mirroring thetransition,duration, andtimingoptions of vanilla Flowbite'sDismissclass (see discussion #1091).This closes the gap called out in #1092 between vanilla Flowbite's Alert and the React component, where the React
<Alert>had no way to configure its dismiss animation.New props
transitionstring(Tailwind class)transition-opacitydurationnumber(ms)300timingstring(Tailwind class)ease-outonDismissnow fires after the transition has finished, or immediately whendurationis0(preserving the previous synchronous behaviour). The transition classes are also exposed via the optionalalert.transitiontheme key for global overrides.Motivation
Users currently have no way to animate the dismissal of an
<Alert>; it disappears instantly. Vanilla Flowbite exposes these options and they were requested back in #1091/#1092.Related issues
Closes #1092.
Type of change
Breaking changes
None. All three new props are optional with defaults that match the existing instant-dismiss behaviour when
durationis0, and the existingonDismisssignature is unchanged.Checklist
bun run format/bun run lint/bun run typecheck/bun run test:coverage/bun run buildall pass locallyonDismiss, immediate dismiss whenduration === 0)alert.transition.tsx)Implementation notes for reviewers
A few decisions worth flagging — happy to adjust if any of them don't fit the project's conventions:
alert.transitionis an optional theme key (no base defaults intheme.ts). The defaults live as prop defaults on the component, and the theme key only wins when a user explicitly sets it. This keeps the prop API authoritative while still allowing global overrides via<ThemeProvider>.onDismissis invoked from asetTimeoutand therefore receives a syntheticMouseEvent(new MouseEvent("click")) rather than the original click event. This keeps the existingonDismiss: (event) => voidsignature unchanged (no breaking change), but if you'd prefer I could split this into an explicit lifecycle (e.g.onDismissStart/onDismiss) instead. Let me know.twMergeso users can override any oftransition/duration-[Nms]/timingindependently.Notes on local test run
bun run test:coverage(the CI command) passes: 344 passed, 1 skipped, 1 todo. The standalonebun test src/helpersjob has one pre-existing failure (withoutThemingProps > should remove theme, clearTheme and applyTheme props) caused by a mock leak fromresolve-props.test.ts— this failure is present onmaintoo and is unrelated to this change.Summary by CodeRabbit
New Features
onDismissnow triggers after the dismissal animation completes, or immediately when duration is set to 0.Documentation
Examples
Tests