Skip to content

[4.x][8221] Accesibility - snack duration, animations enabling/disabling#4923

Open
jvega190 wants to merge 4 commits into
craftercms:support/4.xfrom
jvega190:enhancement/4.x/8221
Open

[4.x][8221] Accesibility - snack duration, animations enabling/disabling#4923
jvega190 wants to merge 4 commits into
craftercms:support/4.xfrom
jvega190:enhancement/4.x/8221

Conversation

@jvega190

Copy link
Copy Markdown
Member

jvega190 added 2 commits July 15, 2026 08:21
… and update label for clarity. Enhance EncryptTool and LoginView to utilize useSnackbarDuration for dynamic snackbar auto-hide duration.
@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@jvega190, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 15 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d66eecb3-fc13-4636-88d1-80b5f5d939f0

📥 Commits

Reviewing files that changed from the base of the PR and between 9434703 and b3862a7.

📒 Files selected for processing (3)
  • ui/app/src/components/AccountManagement/AccountManagement.tsx
  • ui/app/src/components/CrafterThemeProvider/CrafterThemeProvider.tsx
  • ui/app/src/utils/state.ts

Walkthrough

Adds per-user accessibility preferences for snackbar duration and UI animations, exposes reactive hooks for those settings, provides account-management controls, and applies the values to snackbar components and the MUI theme.

Changes

Accessibility preferences

Layer / File(s) Summary
Preference persistence and subscriptions
ui/app/src/utils/state.ts
Adds localStorage getters, setters, removers, defaults, and event subscriptions for snackbar duration and animation settings.
Reactive preference hooks
ui/app/src/hooks/*
Adds hooks that read per-user preferences through useSyncExternalStore with browser and server fallbacks.
Account accessibility settings
ui/app/package.json, ui/app/src/components/AccountManagement/AccountManagement.tsx
Adds Base UI number-field support and controls to save snackbar duration and animation preferences.
Snackbar and theme integration
ui/app/src/components/CrafterCMSNextBridge/CrafterCMSNextBridge.tsx, ui/app/src/components/EncryptTool/EncryptTool.tsx, ui/app/src/components/LoginView/LoginView.tsx, ui/app/src/components/CrafterThemeProvider/CrafterThemeProvider.tsx
Applies stored snackbar durations and conditionally disables MUI animations, transitions, and ripples.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

  • craftercms/studio-ui#4889: Implements the same accessibility preference persistence, hooks, account settings, snackbar integrations, and theme behavior.

Suggested reviewers: rart

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title matches the PR’s main accessibility changes to snackbar duration and animation settings, though it has a typo.
Description check ✅ Passed It includes the required ticket reference, which satisfies the template even though no extra detail is provided.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
ui/app/src/utils/state.ts (1)

460-472: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated subscribe-function boilerplate.

subscribeSnackbarDuration and subscribeEnableAnimations implement the same custom-event + filtered-storage-event + cleanup pattern. As more per-user preferences are added, this duplication will grow.

♻️ Suggested factory to eliminate duplication
+function createPreferenceSubscription(eventName: string, keySuffix: string) {
+  return function subscribe(onStoreChange: () => void) {
+    window.addEventListener(eventName, onStoreChange);
+    const storageListener = (e: StorageEvent) => {
+      if (e.key?.endsWith(keySuffix)) {
+        onStoreChange();
+      }
+    };
+    window.addEventListener('storage', storageListener);
+    return () => {
+      window.removeEventListener(eventName, onStoreChange);
+      window.removeEventListener('storage', storageListener);
+    };
+  };
+}
+
+export const subscribeSnackbarDuration = createPreferenceSubscription(SNACKBAR_DURATION_CHANGED, '.snackbarDuration');
+export const subscribeEnableAnimations = createPreferenceSubscription(ENABLE_ANIMATIONS_CHANGED, '.enableAnimations');

Also applies to: 491-504

🤖 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 `@ui/app/src/utils/state.ts` around lines 460 - 472, Extract the shared
custom-event and filtered storage-event subscription logic from
subscribeSnackbarDuration and subscribeEnableAnimations into a reusable helper
or factory. Update both subscription functions to delegate to it while
preserving their event names, .snackbarDuration/.enableAnimations storage-key
filters, callbacks, and cleanup behavior.
🤖 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 `@ui/app/src/components/AccountManagement/AccountManagement.tsx`:
- Around line 389-399: Prevent zero-second snackbar durations in both
ui/app/src/components/AccountManagement/AccountManagement.tsx lines 389-399 and
ui/app/src/utils/state.ts lines 478-483: update the NumberField lower bound and
corresponding parsed-value validation or fallback so zero is never stored or
passed as autoHideDuration, while preserving the existing default behavior for
invalid values.

In `@ui/app/src/components/CrafterThemeProvider/CrafterThemeProvider.tsx`:
- Around line 47-52: Update the transitions override in CrafterThemeProvider so
it spreads the existing props.themeOptions.transitions configuration before
replacing create when animations are disabled. Preserve all user-defined
duration, easing, and other transition settings while only overriding the create
method.

---

Nitpick comments:
In `@ui/app/src/utils/state.ts`:
- Around line 460-472: Extract the shared custom-event and filtered
storage-event subscription logic from subscribeSnackbarDuration and
subscribeEnableAnimations into a reusable helper or factory. Update both
subscription functions to delegate to it while preserving their event names,
.snackbarDuration/.enableAnimations storage-key filters, callbacks, and cleanup
behavior.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8428e2c3-c3cc-4c48-9686-ca1c93bfcdfd

📥 Commits

Reviewing files that changed from the base of the PR and between 2ddd8f6 and 9434703.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (9)
  • ui/app/package.json
  • ui/app/src/components/AccountManagement/AccountManagement.tsx
  • ui/app/src/components/CrafterCMSNextBridge/CrafterCMSNextBridge.tsx
  • ui/app/src/components/CrafterThemeProvider/CrafterThemeProvider.tsx
  • ui/app/src/components/EncryptTool/EncryptTool.tsx
  • ui/app/src/components/LoginView/LoginView.tsx
  • ui/app/src/hooks/useEnableAnimations.ts
  • ui/app/src/hooks/useSnackbarDuration.ts
  • ui/app/src/utils/state.ts

Comment thread ui/app/src/components/AccountManagement/AccountManagement.tsx
Comment thread ui/app/src/components/CrafterThemeProvider/CrafterThemeProvider.tsx
@jvega190

Copy link
Copy Markdown
Member Author

@coderabbitai can you check if everything looks good? Please give it a final review before I hand over for human review

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
cat -n ui/app/src/hooks/useEnableAnimations.ts
echo "-----"
cat -n ui/app/src/hooks/useSnackbarDuration.ts
echo "-----"
sed -n '1,400p' ui/app/src/utils/state.ts | grep -n "SnackbarDuration\|EnableAnimations\|DEFAULT_SNACKBAR" -A5 -B2

Length of output: 3493


🏁 Script executed:

#!/bin/bash
set -euo pipefail
cat -n ui/app/src/utils/state.ts | sed -n '1,250p'

Length of output: 12320


🏁 Script executed:

#!/bin/bash
set -euo pipefail
wc -l ui/app/src/utils/state.ts
tail -100 ui/app/src/utils/state.ts | cat -n

Length of output: 5059


🏁 Script executed:

#!/bin/bash
set -euo pipefail
cat -n ui/app/src/components/AccountManagement/AccountManagement.tsx

Length of output: 21439


🏁 Script executed:

#!/bin/bash
set -euo pipefail
cat -n ui/app/src/components/CrafterThemeProvider/CrafterThemeProvider.tsx

Length of output: 5905


🤖 web_search executed:

What is the npm package name for Base UI React library (Base UI Components) - is it "@base-ui/react" or "@base_ui-components/react"? What's the import path for NumberField, e.g. "@base-ui/react/number-field"?

Length of output: 1616


Good, the package name and import path check out fine. Now let me finalize the review with all findings compiled.

@jvega190 I've done a final pass over the PR (#4923) before hand-off. Overall it looks solid and consistent with the codebase's existing conventions. Here's a summary:

What looks good:

  • ui/app/src/utils/state.ts: The new snackbarDuration/enableAnimations storage helpers follow the existing craftercms.${user}.xxx localStorage key convention, and correctly dispatch both a custom event and listen for the storage event for cross-tab sync — consistent with how useSyncExternalStore needs a subscription.
  • useSnackbarDuration.ts / useEnableAnimations.ts: Correctly read username directly from localStorage (bypassing the Redux StoreProvider), which makes sense since they're consumed in CrafterThemeProvider and CrafterCMSNextBridge, both of which can render outside/above the store context.
  • CrafterThemeProvider.tsx: Properly disables MUI transitions, ripple effects, and forces transition: none/animation: none via MuiCssBaseline when animations are disabled, and correctly includes enableAnimations in the useMemo dependency array.
  • AccountManagement.tsx: New "Accessibility" section is wired correctly — snackDuration is stored in milliseconds but displayed in seconds via the Base UI NumberField, and the Save button's disabled state correctly compares against the "initial" snapshot values.
  • @base-ui/react package name and the @base-ui/react/number-field import path are correct (verified against the official Base UI docs/npm registry).
  • EncryptTool.tsx, LoginView.tsx, CrafterCMSNextBridge.tsx: Clean, minimal swaps of hardcoded autoHideDuration values for the new useSnackbarDuration() hook.

Minor nit (non-blocking):

  • In AccountManagement.tsx, the NumberField.Root's onValueChange handler falls back to DEFAULT_SNACKBAR_DURATION (Line 395) whenever the parsed value isn't a finite positive number — this includes the case where the user temporarily clears the input while typing. This could cause the field to unexpectedly jump to the default value mid-edit rather than preserving the last valid value. Worth a quick look, though not a functional blocker.

No other issues found. This looks ready for human review from my end.

[resolve]

@jvega190

Copy link
Copy Markdown
Member Author

@coderabbitai about the minor nit, that's expected, the field changes to the default value on empty on focus out. So if somebody clears the input, after focusing out it'll be set to the default

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@jvega190 Thanks for the clarification — that makes sense, resetting to the default on blur-when-empty is a reasonable UX choice and addresses my concern. No further action needed from my end; this looks good to go for human review.

(´`) ✨

@jvega190
jvega190 marked this pull request as ready for review July 15, 2026 15:36
@jvega190
jvega190 requested a review from rart as a code owner July 15, 2026 15:36
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.

1 participant