🪒 feat: Tighten Read-Only UX in Disabled Config Sections#76
Open
dustinhealy wants to merge 2 commits into
Open
🪒 feat: Tighten Read-Only UX in Disabled Config Sections#76dustinhealy wants to merge 2 commits into
dustinhealy wants to merge 2 commits into
Conversation
The config editor previously left a number of interactive affordances visible in fully read-only sections, where the disabled state was either ambiguous, misleading, or actively broken. ConfigRow no longer applies a row-level pointer-events-none when disabled. The catch-all overreached into the SectionHeader (blocking caret expansion and other purely visual controls) and is now unnecessary because every value control inside SectionControls self-blocks via its own disabled attribute. The inline ToggleField at renderInlineField missed its disabled prop, so inline toggles in disabled sections still actuated and silently mutated editedValues without triggering the save bar. Fixed. BooleanChip is the disabled-state stand-in for ToggleField, but the SectionControls slot is a flex column with default align-items stretch, so the chip rendered full-width across the value area. Added self-start so it sits at its natural pill width. NestedGroup gains a disabled prop. When set, the caret/collapse affordance is dropped and the group renders flat with a static heading. Carets in fully read-only sections were misleading because they hinted at interactivity that no longer mattered. Threaded through the four NestedGroup call sites in FieldRenderer plus the More settings group in EndpointsRenderer. ListField now swaps native select for a static span when disabled and the field has enum options. Browsers render their own dropdown caret on disabled selects which conflicted with the new flat read-only look. CustomEndpointsRenderer and McpServersRenderer hide their Create button entirely when disabled rather than rendering an inert button. When disabled and there are no entries, a muted No custom endpoints configured or No MCP servers configured placeholder is shown so the expanded section is not just empty space. Two new English locale keys were added; locize-i18n-sync handles the rest. No behavior change for users with write capability on a section. All removals are gated on the existing disabled prop.
Contributor
Author
|
@codex review |
1 similar comment
Contributor
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60ea4ddaa2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two P3 codex findings on the read-only UX commit. McpServersRenderer rendered the generic com_config_no_entries fallback even when the new disabled-and-empty readonly message also fired, producing duplicate empty states; gating the generic fallback on !disabled matches the same pattern EndpointsRenderer uses. The NestedGroup disabled branch lost the data-section-id={sectionId} attribute that the enabled branch carries on its toggle button, so read-only nested sections dropped out of useActiveSection's scroll-tracking scan even though they remained reachable by id; restoring the attribute on the disabled header div keeps active-section highlighting consistent between read-only and editable sections.
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cleans up the disabled-state UX in the config editor. The current behavior leaves a number of interactive affordances visible in fully read-only sections where the disabled state is ambiguous, misleading, or silently broken: a row-level
pointer-events-noneblocks legitimate header carets, an inlineToggleFieldstill actuates and mutates state without surfacing the save bar, theBooleanChipstand-in stretches edge-to-edge in its flex slot, native disabled<select>controls keep their browser caret in a flat read-only view, expand carets on nested groups hint at write affordances that no longer exist, and the MCP and Custom Endpoints sections render inert "+ Create" buttons against a blank list when the user has no write capability.This PR replaces each of those with cleaner read-only stand-ins. No behavior change for users who hold write capability on a section; all changes are gated on the existing
disabledprop.Change Type
Implementation
ConfigRow: drop row-levelpointer-events-nonewhen disabled. The catch-all overreached into theSectionHeader(blocking carets and other purely visual controls) and is unnecessary now that every value control insideSectionControlsself-blocks via its owndisabledattribute. Audited everycontrolTypebranch inFieldRendererand every component insrc/components/configuration/fields/to confirm.FieldRenderer(inlineToggleField): was missing thedisabledprop. Inline toggles in disabled sections still actuated and silently mutatededitedValueswithout triggering the save bar — the change registered as dirty against a tab the user could not save. Fixed.FieldRenderer(BooleanChip): addedself-start.SectionControlsis aflex flex-colslot with defaultalign-items: stretch, which made the chip span the full value area. Now sits at its natural pill width likeToggleFielddoes.FieldRenderer(NestedGroup): gains adisabledprop. When set, the caret/collapse button is dropped and the group renders flat with a static heading. Carets in fully read-only sections hinted at interactivity that no longer mattered. Threaded through all fourNestedGroupcall sites inFieldRendererplus the "More settings" group inEndpointsRenderer.fields/ListField: when disabled and the field has enumoptions, swap the native<select>for a static<span>showing the matched option label. Browsers render their own dropdown caret on disabled<select>regardless of any CSS, which conflicted with the new flat read-only look.sections/EndpointsRendererandsections/McpServersRenderer: hide the "+ Create endpoint" / "+ Create MCP server" button entirely when disabled rather than rendering an inert button next to a read-only list. When disabled and the list is empty, render a muted "No custom endpoints configured" / "No MCP servers configured" placeholder so the expanded section is not just empty space.locales/en/translation.json: two new keys (com_config_no_custom_endpoints,com_config_no_mcp_servers).locize-i18n-synchandles the rest of the languages.Testing
bun run lint: clean.npx tsc --noEmit: clean.BooleanChippills sit at natural pill width, list-of-enum fields render as static text, empty-section placeholders appear in the expected places.Checklist
Note
Low Risk
UI-only changes gated on the existing
disabledprop; no auth, persistence, or API behavior changes.Overview
Improves how read-only config sections look and behave when
disabledis set, without changing the editable path for users who can write.ConfigRowno longer applies row-levelpointer-events-noneon disabled rows, so header affordances are not blocked while individual controls still honordisabled.FieldRendererthreadsdisabledinto inlineToggleField(fixes toggles that could still mutate state),NestedGroup(flat static heading instead of expand/collapse), andBooleanChip(self-startso pills do not stretch full width).ListFieldshows enum values as static text instead of a disabled<select>with a dropdown caret.Custom Endpoints and MCP Servers hide create buttons when disabled; empty read-only lists show new muted placeholders (
com_config_no_custom_endpoints,com_config_no_mcp_servers). MCP still shows the generic “no entries” hint only when editable and empty.Reviewed by Cursor Bugbot for commit 18631e7. Bugbot is set up for automated code reviews on this repo. Configure here.