Skip to content

fix: segmented_control indicator renders for >10 items#6344

Open
FarhanAliRaza wants to merge 2 commits intoreflex-dev:mainfrom
FarhanAliRaza:pagination-fix
Open

fix: segmented_control indicator renders for >10 items#6344
FarhanAliRaza wants to merge 2 commits intoreflex-dev:mainfrom
FarhanAliRaza:pagination-fix

Conversation

@FarhanAliRaza
Copy link
Copy Markdown
Collaborator

All Submissions:

Radix Themes 3.3.0 ships hardcoded nth-child rules for the sliding
indicator up to 10 items only (radix-ui/themes#730); beyond that the
indicator collapses. Expose item count and selected index as CSS
custom properties on the root and override width/transform with one
calc()-driven rule, supporting any number of items.

  • Have you followed the guidelines stated in CONTRIBUTING.md file?
  • Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

New Feature Submission:

  • Does your submission pass the tests?
  • Have you linted your code locally prior to submission?

Changes To Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

closes #5651

Radix Themes 3.3.0 ships hardcoded nth-child rules for the sliding
indicator up to 10 items only (radix-ui/themes#730); beyond that the
indicator collapses. Expose item count and selected index as CSS
custom properties on the root and override width/transform with one
calc()-driven rule, supporting any number of items.
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 20, 2026

Greptile Summary

This PR fixes two independent issues: (1) the Radix Themes SegmentedControl indicator collapsing to zero width for more than 10 items by injecting CSS custom properties (--rx-sc-count, --rx-sc-idx) and overriding the indicator's width/transform via calc(), and (2) removing the json5 dependency in favour of native JSON.parse with a custom regex fallback for bare Infinity/NaN tokens.

  • P1: An empty file named 10 was accidentally committed to the repository root and must be removed before merge.
  • P2: NaN float values in state now render as blank (coerced to null) rather than the string "NaN" — a user-visible behaviour change with no release note.
  • P2: The >10 items fix is intentionally skipped for type="multiple" segmented controls; this gap should at least be documented in a comment.

Confidence Score: 4/5

Safe to merge after removing the accidental empty file 10 from the repository root.

One P1 issue (accidental empty file) must be cleaned up; remaining findings are P2 style/documentation concerns that do not block correctness.

The empty 10 file at the repository root must be deleted; segmented_control.py and state.js warrant a second look for the noted edge cases.

Important Files Changed

Filename Overview
10 Accidentally committed empty file named "10" at the repository root — should be deleted.
packages/reflex-components-radix/src/reflex_components_radix/themes/components/segmented_control.py Core bug fix: injects CSS custom properties for item count and selected index to override Radix's hardcoded nth-child indicator rules; fix intentionally omits type="multiple" support.
packages/reflex-base/src/reflex_base/.templates/web/utils/state.js Removes json5 dependency; replaces with native JSON.parse plus a regex fallback that rewrites bare Infinity/-Infinity/NaN tokens — NaN is now coerced to null rather than preserved.
packages/reflex-base/src/reflex_base/constants/installer.py Removes json5 from the JS package dependency list, consistent with the state.js change.
tests/integration/tests_playwright/test_appearance.py Adds a Playwright integration test verifying the segmented control indicator renders correctly with 11 items, including bounding-box alignment checks.
tests/integration/test_computed_vars.py Updates NaN assertion to reflect json5→native JSON.parse behavioral change where NaN is now coerced to null (empty string in UI).

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["SegmentedControlRoot.create()"] --> B{type == 'multiple'?}
    B -->|Yes| Z["super().create() — Radix default rules apply"]
    B -->|No| C["_collect_item_values(children)"]
    C --> D{single Foreach?}
    D -->|Yes| E{iterable is ArrayVar AND\nrender_fn returns SegmentedControlItem?}
    E -->|No| Z
    E -->|Yes| G["iterable.foreach(el => render_fn(el).value)"]
    D -->|No| F{all children are\nSegmentedControlItem with value?}
    F -->|No| Z
    F -->|Yes| H["LiteralArrayVar.create(values)"]
    G --> I["values_var: ArrayVar"]
    H --> I
    I --> J{selected prop present?}
    J -->|No| Z
    J -->|Yes| K["inject --rx-sc-count and --rx-sc-idx\ninto style props"]
    K --> L["super().create()"]
    L --> M["add_style(): override\n.rt-SegmentedControlIndicator\nwidth + transform via calc()"]
Loading

Comments Outside Diff (2)

  1. 10, line 1 (link)

    P1 Accidental empty file committed to repository root

    An empty file named 10 was accidentally added to the root of the repository. This is unintentional and should be removed before merging.

  2. tests/integration/test_computed_vars.py, line 235 (link)

    P2 NaN is now silently coerced to null / empty string

    The previous behaviour preserved NaN as a visible string; the new rewriteBareNonFiniteFloats replaces it with null, which renders as an empty string. This is a user-visible semantic change: any state variable that held float('nan') will now display as blank rather than "NaN". The test update acknowledges this, but there is no deprecation notice or release note capturing the breaking change for users who relied on the old rendering.

Reviews (1): Last reviewed commit: "fix: segmented_control indicator renders..." | Re-trigger Greptile

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Apr 20, 2026

Merging this PR will not alter performance

✅ 9 untouched benchmarks


Comparing FarhanAliRaza:pagination-fix (86e8dba) with main (e3c4c29)

Open in CodSpeed

@FarhanAliRaza FarhanAliRaza requested a review from masenf April 20, 2026 11:34
@adhami3310
Copy link
Copy Markdown
Member

i'm not sure we want to fix this here, ideally this should be fixed upstream

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.

rx.segmented_control.root() with more than 10x .item()

2 participants