Skip to content

Add reduce motion accessibility setting - #2

Draft
srbsingh3 wants to merge 6 commits into
mainfrom
reduce-motion-accessibility-setting
Draft

Add reduce motion accessibility setting#2
srbsingh3 wants to merge 6 commits into
mainfrom
reduce-motion-accessibility-setting

Conversation

@srbsingh3

@srbsingh3 srbsingh3 commented Feb 6, 2026

Copy link
Copy Markdown
Owner

Follows up on zed-industries/zed#48295, scoped down per feedback to ship one primitive at a time.

Summary

  • Adds a reduce_motion setting ("system" | "on" | "off", default: "system")
  • Queries macOS accessibilityDisplayShouldReduceMotion when set to "system"
  • Applies the setting to toast slide-in animations as a first consumer
  • Adds the setting to the Appearance page in Settings UI

Next steps

  • Add platform support for Linux (gtk-enable-animations / portal) and Windows (SPI_GETCLIENTAREAANIMATION)
  • Gate more animation call sites (with_animation is used in ~15 other places: cursor blink, edit predictions, agent panel, spinners, etc.)
  • Consider lifting the check into the animate_in trait itself so all callers get it for free

Test plan

  • Unit tests for serialization, round-trip, aliases, and settings store integration
  • GPUI tests for should_reduce_motion with each variant
  • Manual: set "reduce_motion": "on" and verify toast appears without animation
  • Manual: set "reduce_motion": "system" and toggle macOS System Settings > Accessibility > Display > Reduce motion

Add a user-configurable setting to control UI animations based on
accessibility preferences. The setting supports three modes:

- "system": Follows OS accessibility preferences (default)
- "on": Always reduces motion (disables animations)
- "off": Always enables animations

Platform support includes macOS via NSWorkspace accessibility API,
with a default implementation for other platforms.

The setting is exposed in the Settings UI under Appearance > Motion
and currently applies to toast notification animations. Additional
animations can be updated to respect this setting in future changes.
Shorten the description from "Controls whether animations are
reduced. When set to System, follows your OS accessibility
preference." to "Reduce or disable animations. System uses your OS
preference." for better readability and consistency with other
setting descriptions in the UI.
Remove unwrap_or_default() fallback to enforce explicit setting
configuration. The reduce_motion setting must now always be present
in workspace settings.
Simplify test coverage by consolidating overlapping tests into
data-driven test cases:

- Merge 3 should_reduce_motion variant tests into single test
- Combine 2 deserialization tests into data-driven approach
- Consolidate 3 settings store parsing tests with cases array
- Remove redundant default value test (covered elsewhere)
- Fix alphabetical ordering of pub use statements

Reduces line count from 242 to 167 while maintaining identical
test coverage. Each consolidated test includes context messages
for easier failure diagnosis.
@srbsingh3

Copy link
Copy Markdown
Owner Author

PR Review Summary: Add reduce motion accessibility setting

Files changed: 11 (+305/-36)
Branch: reduce-motion-accessibility-settingmain


Critical Issues (0 found)

No critical issues were identified. The PR is well-structured and follows established codebase patterns.


Important Issues (1 found)

1. Misleading "System" description on non-macOS platforms

[error-handling] crates/settings_ui/src/page_data.rs:1055

The settings UI description says:

"Reduce or disable animations. System uses your OS preference."

However, the should_reduce_motion() platform trait defaults to false on Linux/Windows. Users on those platforms who select "system" will see animations continue to play, with no indication that their OS preference is not being queried.

Recommendation: Update the description to mention macOS-only support for the "system" mode, or add platform-specific implementations for Linux (gtk-enable-animations / portal) and Windows (SPI_GETCLIENTAREAANIMATION). The PR's "Next steps" section already tracks this.


Suggestions (2 found)

1. Consider making ReduceMotionSetting inner field private

[type-design] crates/settings/src/reduce_motion_setting.rs:5

The newtype pub struct ReduceMotionSetting(pub ReduceMotion) exposes the inner field, allowing callers to bypass the should_reduce_motion method and access the raw enum. Since ReduceMotion has no invalid states, this is low risk. However, making it ReduceMotionSetting(ReduceMotion) with a pub fn value(&self) -> ReduceMotion accessor would be slightly more encapsulated. This is a minor style preference.

2. Toast layer animation behavior is untested

[tests] crates/workspace/src/toast_layer.rs:261-267

The conditional animation skip (if reduce_motion { ... } else { ... }) is the only consumer of the new setting in this PR, but it has no test coverage. This is understandable given that toast_layer.rs has zero existing tests and testing element tree structure in GPUI is uncommon. The branching logic is straightforward enough that the risk is low.


Strengths

  • Follows established patterns precisely. The ReduceMotion enum matches the derive set and attribute pattern of ShowDiagnostics, WindowDecorations, and CloseWindowWhenNoItems. The ReduceMotionSetting newtype follows the RegisterSetting pattern used by BaseKeymap.

  • The unwrap() in from_settings is correct. Despite CLAUDE.md saying to avoid unwrap(), the Settings::from_settings trait explicitly documents that it should panic on missing defaults. Every other Settings implementation does the same. The default value "reduce_motion": "system" is present in default.json.

  • Excellent public API. The free function should_reduce_motion(cx: &gpui::App) -> bool provides a clean, ergonomic interface. Consumers don't need to know about ReduceMotionSetting, ReduceMotion, or the get_global machinery.

  • Thorough test suite (7.5/10). Covers default values, deserialization of all valid inputs, rejection of invalid inputs, serialization round-trip, all three setting variants, settings store integration with JSON parsing, and registration ordering edge cases.

  • Clean unsafe FFI. The macOS accessibilityDisplayShouldReduceMotion implementation follows the exact same pattern as the adjacent should_auto_hide_scrollbars, and Objective-C nil messaging semantics provide implicit safety.

  • Smart serde aliases. #[serde(alias = "true")] on On and #[serde(alias = "false")] on Off anticipate a common configuration mistake. Tests correctly verify that bare boolean true/false (without quotes) are rejected.

  • Proper VS Code import handling. Setting reduce_motion: None correctly falls back to the default since VS Code has no equivalent setting.


Recommended Action

  1. Consider updating the settings UI description to indicate that "system" detection currently only works on macOS
  2. Merge — no blocking issues found. This is a well-implemented accessibility feature that integrates cleanly across all layers (platform → GPUI → settings → UI)

Review performed by Claude Code using 4 specialized agents: code-reviewer, pr-test-analyzer, silent-failure-hunter, and type-design-analyzer.

Replace ambiguous "Reduce or disable animations" with "Control UI
animations" and specify that System mode reads from macOS preferences,
making the platform scope clear.
Make the inner ReduceMotion field private and add a public value()
accessor method. This provides better encapsulation and allows for
future changes to the internal representation without breaking the
public API.

Update all test assertions to use the new accessor method instead of
direct field access.
srbsingh3 pushed a commit that referenced this pull request Mar 24, 2026
…stries#51059)

Extract data table modules into separate files

This PR extracts the `tests` and `table_row` modules from
`data_table.rs` into separate files to improve code organization. This
is preparatory work for the upcoming column width API rework (#2 in the
series), where separating mechanical changes from logical changes will
make the review easier.

The extraction was performed using rust-analyzer's "Extract module to
file" command.

**Context:**

This is part 1 of a 3-PR series improving data table column width
handling:
1. **This PR**: Extract modules into separate files (mechanical change)
2. [zed-industries#51060](zed-industries#51060) -
Introduce width config enum for redistributable column widths (API
rework)
3. Implement independently resizable column widths (new feature)

The series builds on previously merged infrastructure:
- [zed-industries#46341](zed-industries#46341) - Data
table dynamic column support
- [zed-industries#46190](zed-industries#46190) - Variable
row height mode for data tables

Primary beneficiary: CSV preview feature
([zed-industries#48207](zed-industries#48207))

Release Notes:

- N/A
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