chore: add <SettingsGroup> to auto-group settings blocks#593
Merged
SupertigerDev merged 1 commit intoNerimity:mainfrom Mar 2, 2026
Merged
chore: add <SettingsGroup> to auto-group settings blocks#593SupertigerDev merged 1 commit intoNerimity:mainfrom
SupertigerDev merged 1 commit intoNerimity:mainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a new SettingsGroup export and composes many settings panels with SettingsGroup/Block; refactors SettingsBlock radius/header handling into CSS module classes; Block gains optional style and onClick props. No substantive state or behavioral changes. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/ui/settings-block/Block.tsx`:
- Around line 17-25: The Block component currently renders a div with
props.onClick but lacks keyboard and semantic affordances; update the render to
detect when props.onClick is present and: add role="button" and tabIndex={0},
attach an onKeyDown handler that triggers props.onClick when Enter or Space is
pressed, and add a visual affordance CSS class (e.g., styles.clickable or
similar) via the existing classNames call so focus and hover styles indicate
interactivity; ensure you use the existing symbols (Block component rendering
the div, props.onClick, classNames, conditionalClass, styles.block/
styles.joinTop/ styles.joinBottom, props.class, props.style) so the change is
localized and consistent.
In `@src/components/ui/settings-block/SettingsBlock.tsx`:
- Around line 86-100: The GroupProps type declares a style prop but it isn't
applied in SettingsGroup; either forward it to the rendered div (add
style={props.style} on the root div in SettingsGroup and ensure you're using
props.children directly rather than the strange children(() => props.children)
helper) or remove style from GroupProps if you intentionally don't want inline
styles; update GroupProps and the SettingsGroup implementation accordingly
(references: GroupProps, SettingsGroup, props.style, props.children).
In `@src/components/ui/settings-block/styles.module.scss`:
- Around line 29-39: The current grouping CSS uses .group >
.block:not(:first-child) and :not(:last-child), which considers position among
all siblings and incorrectly removes border radii when non-.block siblings
(e.g., tokenCategory divs) are present; update the rules to use adjacent sibling
combinators so only consecutive .block elements are joined (replace the
:not(:first-child) and :not(:last-child) conditions with selectors like .group >
.block + .block and .group > .block + .block { /* bottom-radius removal for
preceding block or top-radius removal for following block as appropriate */ } to
target only back-to-back .block elements and preserve radii around other
siblings).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/components/settings/ActivityStatus.tsxsrc/components/settings/BadgeSettings.tsxsrc/components/settings/CallSettings.tsxsrc/components/settings/ConnectionsSettings.tsxsrc/components/settings/InterfaceSettings.tsxsrc/components/settings/NotificationsSettings.tsxsrc/components/settings/PrivacySettings.tsxsrc/components/settings/WindowSettings.tsxsrc/components/ui/settings-block/Block.tsxsrc/components/ui/settings-block/SettingsBlock.tsxsrc/components/ui/settings-block/styles.module.scss
Comment on lines
+17
to
+25
| <div | ||
| onClick={props.onClick} | ||
| class={classNames( | ||
| styles.block, | ||
| conditionalClass(props.borderTopRadius === false, css`&& {border-top-left-radius: 0; border-top-right-radius: 0; margin-top: 0;}`), | ||
| conditionalClass(props.borderBottomRadius === false, css`&& {border-bottom-left-radius: 0; border-bottom-right-radius: 0; margin-bottom: 0;}`), | ||
| conditionalClass(props.borderBottomRadius === false && props.borderTopRadius === false, css`&& {margin-bottom: 1px;}`), | ||
| conditionalClass(props.borderTopRadius === false, styles.joinTop), | ||
| conditionalClass(props.borderBottomRadius === false, styles.joinBottom), | ||
| props.class | ||
| )}> | ||
| )} | ||
| style={props.style} |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the Block.tsx file to understand the full context
cat -n src/components/ui/settings-block/Block.tsxRepository: Nerimity/nerimity-web
Length of output: 1081
🏁 Script executed:
# Check if styles.clickable exists in the styles file
fd -e "css" -e "scss" -e "module.css" -e "module.scss" settings-block | head -20Repository: Nerimity/nerimity-web
Length of output: 47
🏁 Script executed:
# Search for any keyboard handling patterns in the codebase
rg "onKeyDown|onKeyPress|role.*button" src/components --type tsx --type ts -A 2 | head -50Repository: Nerimity/nerimity-web
Length of output: 92
🏁 Script executed:
# Check the styles file for Block component
cat -n src/components/ui/settings-block/styles.module.scssRepository: Nerimity/nerimity-web
Length of output: 1675
🏁 Script executed:
# Search for keyboard handling in tsx/ts files with correct type flag
rg "onKeyDown|onKeyPress|role.*button" src/components -t ts -A 2 | head -60Repository: Nerimity/nerimity-web
Length of output: 4130
🏁 Script executed:
# Look for other accessible button-like patterns in the codebase
rg "role=.*button" src/components -t ts -B 1 -A 3 | head -50Repository: Nerimity/nerimity-web
Length of output: 47
🏁 Script executed:
# Check if there are any existing accessible interactive components as reference
fd -e "module.scss" settings-blockRepository: Nerimity/nerimity-web
Length of output: 47
🏁 Script executed:
# Check how Block component is used with onClick
rg "Block.*onClick|onClick.*=.*\{" src/components -t ts -B 1 -A 2 | head -40Repository: Nerimity/nerimity-web
Length of output: 2472
🏁 Script executed:
# Check imports of Block to see usage frequency
rg "from.*settings-block.*Block|import.*Block.*from" src -t ts | head -20Repository: Nerimity/nerimity-web
Length of output: 2614
🏁 Script executed:
# Look for other button/interactive components to see patterns
fd -type f -name "*.tsx" src/components | xargs grep -l "onClick" | head -10Repository: Nerimity/nerimity-web
Length of output: 235
🏁 Script executed:
# Search for actual Block component usage with onClick
rg "Block.*onClick|<Block" src/components -t ts -A 3 -B 1 | head -50Repository: Nerimity/nerimity-web
Length of output: 2985
🏁 Script executed:
# Check specific files that import Block for usage patterns
grep -n "onClick" src/components/settings/ActivityStatus.tsx src/components/settings/PrivacySettings.tsx src/components/settings/NotificationsSettings.tsxRepository: Nerimity/nerimity-web
Length of output: 1209
Clickable Block needs keyboard semantics and interactive affordance.
onClick on a plain div is currently mouse-centric and inaccessible to keyboard users. Add keyboard activation (Enter/Space), focusability, semantic role, and visual affordance when onClick is provided.
Proposed fix
<div
onClick={props.onClick}
+ role={props.onClick ? "button" : undefined}
+ tabIndex={props.onClick ? 0 : undefined}
+ onKeyDown={(e) => {
+ if (!props.onClick) return;
+ if (e.key === "Enter" || e.key === " ") {
+ e.preventDefault();
+ props.onClick();
+ }
+ }}
class={classNames(
styles.block,
+ conditionalClass(props.onClick, styles.clickable),
conditionalClass(props.borderTopRadius === false, styles.joinTop),
conditionalClass(props.borderBottomRadius === false, styles.joinBottom),
props.class
)}
style={props.style}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div | |
| onClick={props.onClick} | |
| class={classNames( | |
| styles.block, | |
| conditionalClass(props.borderTopRadius === false, css`&& {border-top-left-radius: 0; border-top-right-radius: 0; margin-top: 0;}`), | |
| conditionalClass(props.borderBottomRadius === false, css`&& {border-bottom-left-radius: 0; border-bottom-right-radius: 0; margin-bottom: 0;}`), | |
| conditionalClass(props.borderBottomRadius === false && props.borderTopRadius === false, css`&& {margin-bottom: 1px;}`), | |
| conditionalClass(props.borderTopRadius === false, styles.joinTop), | |
| conditionalClass(props.borderBottomRadius === false, styles.joinBottom), | |
| props.class | |
| )}> | |
| )} | |
| style={props.style} | |
| <div | |
| onClick={props.onClick} | |
| role={props.onClick ? "button" : undefined} | |
| tabIndex={props.onClick ? 0 : undefined} | |
| onKeyDown={(e) => { | |
| if (!props.onClick) return; | |
| if (e.key === "Enter" || e.key === " ") { | |
| e.preventDefault(); | |
| props.onClick(); | |
| } | |
| }} | |
| class={classNames( | |
| styles.block, | |
| conditionalClass(props.onClick, styles.clickable), | |
| conditionalClass(props.borderTopRadius === false, styles.joinTop), | |
| conditionalClass(props.borderBottomRadius === false, styles.joinBottom), | |
| props.class | |
| )} | |
| style={props.style} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/settings-block/Block.tsx` around lines 17 - 25, The Block
component currently renders a div with props.onClick but lacks keyboard and
semantic affordances; update the render to detect when props.onClick is present
and: add role="button" and tabIndex={0}, attach an onKeyDown handler that
triggers props.onClick when Enter or Space is pressed, and add a visual
affordance CSS class (e.g., styles.clickable or similar) via the existing
classNames call so focus and hover styles indicate interactivity; ensure you use
the existing symbols (Block component rendering the div, props.onClick,
classNames, conditionalClass, styles.block/ styles.joinTop/ styles.joinBottom,
props.class, props.style) so the change is localized and consistent.
Comment on lines
+29
to
39
| .group > .block { | ||
| &:not(:first-child) { | ||
| border-top-left-radius: 0; | ||
| border-top-right-radius: 0; | ||
| margin-top: 0; | ||
| } | ||
| &:not(:last-child) { | ||
| border-bottom-left-radius: 0; | ||
| border-bottom-right-radius: 0; | ||
| margin-bottom: 1px; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine the SCSS file
fd -t f "styles.module.scss" | grep -i "settings-block"Repository: Nerimity/nerimity-web
Length of output: 116
🏁 Script executed:
# Then check the InterfaceSettings.tsx file
fd -t f "InterfaceSettings.tsx"Repository: Nerimity/nerimity-web
Length of output: 110
🏁 Script executed:
# Read the SCSS file to understand the full context and selector logic
cat -n src/components/ui/settings-block/styles.module.scssRepository: Nerimity/nerimity-web
Length of output: 1675
🏁 Script executed:
# Read InterfaceSettings.tsx around line 333 to see the actual sibling structure
sed -n '320,350p' src/components/settings/InterfaceSettings.tsxRepository: Nerimity/nerimity-web
Length of output: 1067
🏁 Script executed:
# Search for SettingsGroup usage and structure to understand the DOM hierarchy
rg -A 5 "SettingsGroup" src/components/settings/InterfaceSettings.tsx | head -100Repository: Nerimity/nerimity-web
Length of output: 981
🏁 Script executed:
# Also search for how tokenCategory and SettingsBlock are nested
rg "tokenCategory" src/components/settings/InterfaceSettings.tsx -B 2 -A 2Repository: Nerimity/nerimity-web
Length of output: 211
🏁 Script executed:
# Read the SettingsBlock component to understand its structure
cat -n src/components/ui/settings-block/SettingsBlock.tsxRepository: Nerimity/nerimity-web
Length of output: 3670
🏁 Script executed:
# Search for the specific section with tokenCategory to see the full structure
sed -n '320,360p' src/components/settings/InterfaceSettings.tsxRepository: Nerimity/nerimity-web
Length of output: 1416
Use adjacent sibling combinators instead of :first-child/:last-child for block grouping.
The selectors .group > .block:not(:first-child) and :not(:last-child) evaluate position among all sibling elements, not just .block elements. The tokenCategory divs in src/components/settings/InterfaceSettings.tsx (lines 331–334) are direct siblings to blocks within .group, which causes the CSS to incorrectly strip border radii from blocks following these non-block siblings. Use adjacent sibling combinators (e.g., .block + .block) to only join consecutive .block elements.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/settings-block/styles.module.scss` around lines 29 - 39,
The current grouping CSS uses .group > .block:not(:first-child) and
:not(:last-child), which considers position among all siblings and incorrectly
removes border radii when non-.block siblings (e.g., tokenCategory divs) are
present; update the rules to use adjacent sibling combinators so only
consecutive .block elements are joined (replace the :not(:first-child) and
:not(:last-child) conditions with selectors like .group > .block + .block and
.group > .block + .block { /* bottom-radius removal for preceding block or
top-radius removal for following block as appropriate */ } to target only
back-to-back .block elements and preserve radii around other siblings).
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/ui/settings-block/Block.tsx (1)
17-26:⚠️ Potential issue | 🟠 MajorAdd keyboard semantics for clickable
Block.
onClickon a plaindivis mouse-only right now. Add button semantics (role,tabIndex,Enter/Spacehandling) and a clickable affordance class whenprops.onClickis set.Proposed fix
<div onClick={props.onClick} + role={props.onClick ? "button" : undefined} + tabIndex={props.onClick ? 0 : undefined} + onKeyDown={(e) => { + if (!props.onClick) return; + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + props.onClick(); + } + }} class={classNames( styles.block, + conditionalClass(props.onClick, styles.clickable), conditionalClass(props.borderTopRadius === false, styles.joinTop), conditionalClass(props.borderBottomRadius === false, styles.joinBottom), props.class )} style={props.style}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/settings-block/Block.tsx` around lines 17 - 26, The Block component currently only attaches props.onClick to a div, so add keyboard/button semantics and a visual affordance when clickable: when props.onClick is present, add role="button" and tabIndex={0}, include a clickable class (e.g., styles.clickable) in the classNames call, and implement an onKeyDown handler on the same element that calls props.onClick() when the user presses Enter or Space (for Space call preventDefault to avoid scrolling); keep existing onClick, classNames, conditionalClass, props.style and props.class usage intact so behavior is identical for mouse but accessible via keyboard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/ui/settings-block/Block.tsx`:
- Around line 17-26: The Block component currently only attaches props.onClick
to a div, so add keyboard/button semantics and a visual affordance when
clickable: when props.onClick is present, add role="button" and tabIndex={0},
include a clickable class (e.g., styles.clickable) in the classNames call, and
implement an onKeyDown handler on the same element that calls props.onClick()
when the user presses Enter or Space (for Space call preventDefault to avoid
scrolling); keep existing onClick, classNames, conditionalClass, props.style and
props.class usage intact so behavior is identical for mouse but accessible via
keyboard.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/components/settings/ActivityStatus.tsxsrc/components/settings/BadgeSettings.tsxsrc/components/settings/CallSettings.tsxsrc/components/settings/ConnectionsSettings.tsxsrc/components/settings/InterfaceSettings.tsxsrc/components/settings/NotificationsSettings.tsxsrc/components/settings/PrivacySettings.tsxsrc/components/settings/WindowSettings.tsxsrc/components/ui/settings-block/Block.tsxsrc/components/ui/settings-block/SettingsBlock.tsxsrc/components/ui/settings-block/styles.module.scss
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/settings/NotificationsSettings.tsx
- src/components/ui/settings-block/styles.module.scss
- src/components/settings/ConnectionsSettings.tsx
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/settings/ActivityStatus.tsx (1)
418-436: The negative margin is a visual workaround.The
margin-top: "-1px"on theBlockcontaining theEmojiPickerappears to be compensating for border/gap spacing to visually connect it to the settings block above. This works but is fragile if the underlying styles change.Consider documenting this with a comment or exploring if the
SettingsGroupCSS could handle this edge case natively.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/ActivityStatus.tsx` around lines 418 - 436, The negative margin on the Block wrapping EmojiPicker is a fragile visual workaround; update the code to remove the inline margin-top: "-1px" and either (a) add a short explanatory comment at the Block (near the Block component usage) if keeping it is unavoidable, or (b) better: adjust the surrounding CSS by modifying SettingsGroup/SettingsBlock styles to remove the gap or collapse the border so the picker sits flush (look for SettingsGroup, SettingsBlock, Block, and EmojiPicker to implement the style change), then toggle showEmojiPicker() as before and ensure the visual connection remains without hardcoded negative margins.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/settings/ActivityStatus.tsx`:
- Around line 418-436: The negative margin on the Block wrapping EmojiPicker is
a fragile visual workaround; update the code to remove the inline margin-top:
"-1px" and either (a) add a short explanatory comment at the Block (near the
Block component usage) if keeping it is unavoidable, or (b) better: adjust the
surrounding CSS by modifying SettingsGroup/SettingsBlock styles to remove the
gap or collapse the border so the picker sits flush (look for SettingsGroup,
SettingsBlock, Block, and EmojiPicker to implement the style change), then
toggle showEmojiPicker() as before and ensure the visual connection remains
without hardcoded negative margins.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/components/settings/ActivityStatus.tsxsrc/components/settings/BadgeSettings.tsxsrc/components/settings/CallSettings.tsxsrc/components/settings/ConnectionsSettings.tsxsrc/components/settings/InterfaceSettings.tsxsrc/components/settings/NotificationsSettings.tsxsrc/components/settings/PrivacySettings.tsxsrc/components/settings/WindowSettings.tsxsrc/components/ui/settings-block/Block.tsxsrc/components/ui/settings-block/SettingsBlock.tsxsrc/components/ui/settings-block/styles.module.scss
🚧 Files skipped from review as they are similar to previous changes (5)
- src/components/ui/settings-block/Block.tsx
- src/components/settings/WindowSettings.tsx
- src/components/ui/settings-block/styles.module.scss
- src/components/settings/PrivacySettings.tsx
- src/components/settings/NotificationsSettings.tsx
This was referenced Mar 2, 2026
4 tasks
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
2 participants
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.
This adds a
<SettingsGroup>component to handle styling the grouping of setting blocks automatically, and then refactors the main settings page to use the new groups. This doesn't refactor server settings / application settings / moderation; those are for future PRs.This is also partial prep-work for making the gaps between settings blocks more consistent, but that's also for a future PR. This PR causes some minor changes to the spacing between settings blocks in some cases, but the spacing is already inconsistent, so it shouldn't be much of an issue.
Did you test your code?
Tested on Firefox on Linux and Chrome on Android. Note that this refactors the activity status modal, which only normally appears on the desktop app. I manually enabled it for testing, but I don't know how you'd properly test local changes on the desktop app.
Checklist
Text/content changes support internationalization (i18n)Any new user-facing strings are properly localizedSummary by CodeRabbit