feat/make-dokan-settings-components

[Aunshon]

Summary by CodeRabbit

• New Features

  • New field types and UI controls: Rich Text, Google Analytics, Copy Field, Info, Notice, CombineInput, Color Picker, CopyInput (copy-to-clipboard), and enhanced radio option cards (image/icon). Storybook stories added.

• Improvements

  • Better field layouts (horizontal/full-width), nested/group rendering, danger-state styling, icon/image support, tooltips, doc-link rendering, scrollable settings, richer previews, and minor accessibility/UX tweaks (pointer cursor on switches).

• Chores

  • React peer dependency relaxed to support v17+.

[coderabbitai]

Warning

Rate limit exceeded

@Aunshon has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 11 minutes and 36 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📝 Walkthrough

Walkthrough

Lowered React peerDependency to 17 and added many new settings field variants and UI primitives; propagated isNested/isGroupParent through settings renderers; enabled RawHTML rendering, tooltips, ScrollArea integration, and new Storybook stories.

Changes

Cohort / File(s)	Summary
Dependency Management package.json	Lowered peerDependencies: react and react-dom from >=18.0.0 to >=17.0.0.
Types src/components/settings/settings-types.ts	Added is_danger, doc_link_text, layout, startIcon?, endIcon?; added isNested? and isGroupParent? to FieldComponentProps.
Field Rendering src/components/settings/field-renderer.tsx	Expanded variant dispatch (many new variants including rich_text, google_analytics, combine_input, switch_group, color_picker, notice, copy_field, info, etc.), added isNested/isGroupParent props, recursive child rendering, and fallback handling.
Field Implementations src/components/settings/fields.tsx	Large addition of field components (RichTextField, GoogleAnalyticsField, ColorPickerField, CopyField, CombineInputField, NoticeField, PreviewMulticheckField, FallbackField, etc.), FieldWrapper/FieldLabel enhancements, nested/group layout handling, password visibility toggle, and RawHTML descriptions.
Settings Layout / Rendering src/components/settings/index.tsx, src/components/settings/settings-content.tsx, src/components/settings/settings-formatter.ts	Wrapped titles/descriptions with RawHTML, added ScrollArea and tooltip/info UI, forwarded isNested/isGroupParent through ElementRenderer/SettingsSubSection/SettingsFieldGroup, and adjusted dependency evaluation to consider dependency effect.
UI Primitives — additions & exports src/components/ui/index.ts, src/components/ui/radio-group.tsx, src/components/ui/popover.tsx, src/components/ui/scroll-area.tsx	Exported new components (ColorPicker, CopyInput, CombineInput), added RadioImageCard/RadioIconCard (+ prop types), re-exported PopoverPortal/PopoverClose/PopoverArrow, and added scrollBarClassName support to ScrollArea.
New UI Components src/components/ui/color-picker.tsx, src/components/ui/copy-input.tsx, src/components/ui/combine-input.tsx, src/components/ui/rich-text-editor.tsx	Added ColorPicker (Popover-wrapped WP picker), CopyInput (clipboard helper), CombineInput (paired inputs), and made RichTextEditor controllable via value/onChange.
Stories / Documentation src/components/ui/*.stories.tsx	Added/updated Storybook stories for ColorPicker, CopyInput, CombineInput, and RadioGroup image/icon cards.
Minor styling/behavior tweaks src/components/ui/input.tsx, src/components/ui/popover.tsx, src/components/ui/switch.tsx, src/components/ui/checkbox.tsx	Adjusted classNames (Input, PopoverArrow), added cursor-pointer to Switch, extended LabeledCheckbox with optional image prop.

Sequence Diagram(s)

sequenceDiagram
  participant User as User
  participant UI as SettingsContent
  participant Renderer as FieldRenderer
  participant Field as FieldComponent
  participant Ext as External (WP ColorPicker / Clipboard / Editor)

  rect rgba(200,200,255,0.5)
  User->>UI: open settings / interact
  UI->>Renderer: render element (with isNested/isGroupParent)
  end

  rect rgba(200,255,200,0.5)
  Renderer->>Field: dispatch based on variant
  Field->>Ext: invoke external (open color picker / copy / editor commands)
  Ext-->>Field: result (color / copy confirmation / updated HTML)
  Field-->>Renderer: onChange(value)
  Renderer-->>UI: propagate change
  UI-->>User: UI updates
  end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• Feat/settings components #52: Modifies the same settings subsystem (FieldRenderer, fields, settings-content, settings-types); closely related at code level.
• feat: add Popover, Skeleton, and ScrollArea components with Storybook stories #50: Touches UI primitives (Popover, ScrollArea) and exports that this PR also updates.

Suggested reviewers

• mrabbani

Poem

🐰
I hopped through props and nested trees,
Added colors, copies, and rich-text pleas.
Cards with images, icons that gleam,
React seventeen joins the team.
A nibble, a thump — new UI to dream.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 65.12% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'feat/make-dokan-settings-components' refers to a feature branch format but lacks clarity about the main change. The extensive changeset involves React peer dependency downgrade, new UI components, prop additions, and settings system enhancements, but the title doesn't clearly convey these primary changes.	Consider a more descriptive title like 'Add comprehensive settings components with React 17 support' or 'Enhance settings UI with new field variants and components' that better captures the key changes.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/make-dokan-settings-components

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (7)

src/components/ui/rich-text-editor.tsx (1)

185-246: ⚠️ Potential issue | 🟡 Minor

Duplicate “Link” toolbar button.
There are now two identical Link buttons; this is likely a duplication from inserting the new one without removing the original. Consider keeping just one.

🛠️ Suggested fix (remove the later duplicate)

-        <button
-          type="button"
-          className="p-1.5 hover:bg-muted rounded"
-          title="Link"
-          onClick={() => {
-            const url = prompt("Enter URL:");
-            if (url) executeCommand("createLink", url);
-          }}
-        >
-          <Link className="size-4" />
-        </button>
-
         {variant === "full" && (

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/ui/rich-text-editor.tsx` around lines 185 - 246, Remove the
duplicated Link toolbar button: locate the second <button> with title="Link"
that calls executeCommand("createLink", url) (the one appearing after the second
divider) and delete that entire button block so only the original Link button
remains; ensure you keep the first Link button (the one before the
Quote/Lists/Align group) and that no other handlers or imports (e.g., Link icon)
are removed.

src/components/ui/scroll-area.tsx (1)

10-25: ⚠️ Potential issue | 🟡 Minor

Avoid leaking scrollBarClassName into ScrollAreaPrimitive.Root.
Spreading props passes the custom prop through, which can become an invalid DOM attribute if the primitive forwards props.

🛠️ Suggested fix

-function ScrollArea({
-  className,
-  children,
-  ...props
-}: ScrollAreaRootType) {
+function ScrollArea({
+  className,
+  children,
+  scrollBarClassName,
+  ...props
+}: ScrollAreaRootType) {
   return (
     <ScrollAreaPrimitive.Root
       data-slot="scroll-area"
       className={cn("relative overflow-hidden", className)}
       {...props}
     >
@@
-      <ScrollBar scrollBarClassName={props.scrollBarClassName} />
+      <ScrollBar scrollBarClassName={scrollBarClassName} />

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/ui/scroll-area.tsx` around lines 10 - 25, The current
ScrollArea component spreads all props into ScrollAreaPrimitive.Root which leaks
the custom scrollBarClassName prop; destructure scrollBarClassName out of the
incoming props (e.g., function ScrollArea({ className, children,
scrollBarClassName, ...props }: ScrollAreaRootType)) and then spread only the
remaining props into ScrollAreaPrimitive.Root while passing scrollBarClassName
explicitly to the ScrollBar component to prevent an invalid DOM attribute from
being forwarded.

src/components/settings/index.tsx (1)

116-134: ⚠️ Potential issue | 🟠 Major

Sanitize the title prop before rendering with RawHTML to prevent XSS attacks.

The title is rendered directly via RawHTML without any sanitization at lines 116 and 133. If the title originates from user input, API responses, or any untrusted source, this creates an XSS vulnerability. Either:

1. Sanitize title before passing it to RawHTML (recommended if HTML support is intentional)
2. Change to plain text rendering for the title
3. Document that consumers must sanitize the title before passing it to the Settings component

This pattern appears throughout the Settings component (descriptions, labels, validation errors), so consider applying the same fix broadly.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/index.tsx` around lines 116 - 134, The title prop is
being rendered via RawHTML (see RawHTML usage around the title occurrences in
this Settings component), creating an XSS risk; either sanitize title before
passing it to RawHTML (e.g., run it through a trusted sanitizer like DOMPurify
in the component or a helper and then pass the sanitized string to RawHTML) or
stop using RawHTML and render the title as plain text (replace
<RawHTML>{title}</RawHTML> with a text-only render) in both mobile and desktop
title locations; apply the same change for other RawHTML usages in this
component (descriptions, labels, validation errors) or clearly document that
callers must pass already-sanitized HTML.

src/components/settings/fields.tsx (2)

43-117: ⚠️ Potential issue | 🟠 Major

Avoid rendering runtime validationError as raw HTML (XSS footgun).

validationError comes from runtime validation (errors[...] in FieldRenderer). Treating it as HTML (<RawHTML>) can allow injection if any error message includes user-controlled data. Prefer rendering as plain text (or sanitize at the boundary before storing in errors).

Safer rendering (plain text)

- {element.validationError && (
-   <div className="text-sm text-destructive"><RawHTML>{element.validationError}</RawHTML></div>
- )}
+ {element.validationError && (
+   <div className="text-sm text-destructive">{element.validationError}</div>
+ )}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/fields.tsx` around lines 43 - 117, FieldWrapper
currently renders element.validationError via the RawHTML component which allows
untrusted HTML injection; change both occurrences that wrap
<RawHTML>{element.validationError}</RawHTML> to render plain text (e.g., a
simple div/span with the error string) or ensure the error is sanitized before
insertion so no HTML is interpreted. Update the two places in FieldWrapper (the
full-width branch and the grid branch) to stop using RawHTML and instead render
element.validationError as escaped/plain text, and ensure any tests or callers
(e.g., FieldRenderer) still provide a string rather than HTML.

---

520-645: ⚠️ Potential issue | 🔴 Critical

Remove the image prop from LabeledCheckbox calls (lines 545 and 619).

LabeledCheckbox does not accept an image prop. The description (correctly typed as React.ReactNode) and className are properly supported. Remove image={option.image} from both callsites, or add image support to LabeledCheckboxProps if the design requires it.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/fields.tsx` around lines 520 - 645, The
LabeledCheckbox component is being passed an unsupported image prop; remove
image={option.image} from both MulticheckField and PreviewMulticheckField
LabeledCheckbox callsites (references: MulticheckField, PreviewMulticheckField,
and LabeledCheckbox) so only supported props (checked, onCheckedChange, label,
className, description, etc.) are passed, or alternatively add image support to
LabeledCheckboxProps and its implementation if the design requires images.

src/components/settings/settings-content.tsx (2)

292-327: ⚠️ Potential issue | 🟡 Minor

Make allChildrenAreFields explicitly boolean to avoid undefined semantics.

element.children?.every(...) yields undefined when there are no children; using ?? false makes intent clearer and avoids accidental “no children” being treated as “not all fields”.

Proposed tweak

-const allChildrenAreFields = element.children?.every(
+const allChildrenAreFields = element.children?.every(
     (c) => c.type === 'field' || c.type === 'fieldgroup'
-);
+) ?? false;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/settings-content.tsx` around lines 292 - 327, The
variable allChildrenAreFields in SettingsSubSection can be undefined when
element.children is absent; change its initialization to a strict boolean by
using nullish coalescing (e.g., set allChildrenAreFields =
element.children?.every(...) ?? false) so downstream className decisions and
conditional rendering use a clear true/false value; update the declaration in
the SettingsSubSection function where allChildrenAreFields is defined.

---

51-143: ⚠️ Potential issue | 🟠 Major

Doc link accessibility and save-area implementation gap: must address both.

The doc link can render as icon-only when doc_link_text is empty (or falsy), creating an accessibility issue—add a fallback label and aria-label. More critically, the JSDoc for renderSaveButton states "If not provided but onSave is set, a default icon-only save button is rendered," but the current code renders null instead. This breaks the documented API contract: consumers relying on the default button will see no save functionality. Either implement the default button or update the JSDoc.

• Doc link: add aria-label and fallback text (e.g., 'Docs') when doc_link_text is empty.
• Save button: either provide the documented default implementation or clarify in the JSDoc that renderSaveButton is required when onSave is set.

Suggested fix for doc link accessibility

 {contentSource.doc_link && (
   <a
     href={contentSource.doc_link}
     target="_blank"
     rel="noopener noreferrer"
+    aria-label={contentSource.doc_link_text || 'Documentation'}
     className="text-muted-foreground flex gap-1 items-center text-sm hover:text-foreground transition-colors shrink-0"
   >
     <FileText className="size-4" />
-    { contentSource.doc_link_text ?? '' }
+    {contentSource.doc_link_text || 'Docs'}
   </a>
 )}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/settings-content.tsx` around lines 51 - 143, The
doc-link anchor (contentSource.doc_link and contentSource.doc_link_text) can be
icon-only which is inaccessible, and the component violates its JSDoc by
returning null when renderSaveButton is absent though onSave (handleSave) may
exist; update the anchor to always include an accessible label by using
contentSource.doc_link_text ?? 'Docs' and add an aria-label (e.g.,
aria-label={contentSource.doc_link_text ?? 'Docs'}) alongside the FileText icon,
and implement the documented default save control: when showSaveArea is true and
renderSaveButton is not provided, render a default icon-only save button that
calls handleSave (and reflects dirty state) so consumers get the advertised
fallback, or alternatively update the JSDoc if you prefer requiring
renderSaveButton (but do one or the other to restore contract consistency.

🧹 Nitpick comments (4)

src/components/ui/RadioGroup.stories.tsx (1)

386-453: Consider local fixtures for the image/icon stories.
External image sources can make Storybook previews flaky offline or in CI. Local static assets or data URIs keep the stories deterministic.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/ui/RadioGroup.stories.tsx` around lines 386 - 453, The Image
Card and Icon Card stories (ImageCard, IconCard) use external picsum URLs inside
the Demo functions passed to RadioImageCard and RadioIconCard which makes
Storybook flaky; replace those external image/icon URLs with local static assets
or embedded data URIs and add simple local fixtures (e.g., import
./fixtures/image1.png, image2.png or use small base64 data URIs) and update the
RadioImageCard and RadioIconCard props to use those local sources so the stories
are deterministic in CI/offline.

src/components/settings/settings-types.ts (1)

177-182: isNested / isGroupParent propagation: add a brief doc on semantics.

These flags are now part of the public field component contract; a short comment describing when to set each will prevent inconsistent propagation.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/settings-types.ts` around lines 177 - 182, Add short
doc comments to the FieldComponentProps interface describing the semantics and
propagation rules for isNested and isGroupParent: explain that isNested
indicates the field is rendered inside another field/group (set by parent field
renderers when embedding children), and isGroupParent signals that this field
acts as a container for child fields (set true on group/container components so
children can detect group context); attach these comments directly above the
isNested and isGroupParent properties in FieldComponentProps and mention
expected propagation behavior by parent components and child consumers,
referencing SettingsElement usage where relevant.

src/components/settings/field-renderer.tsx (1)

68-90: switch_group recursion looks good; double-check what isGroupParent is meant to signal for children.

Right now children set isGroupParent={index < length - 1} (i.e., “has next sibling”), while parent SwitchField sets it to “has visible children”. If isGroupParent is purely a spacing flag, consider renaming or documenting it to avoid future misuse.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/field-renderer.tsx` around lines 68 - 90, The usages
of the isGroupParent prop are inconsistent: FieldRenderer passes
isGroupParent={index < length - 1} to child FieldRenderer to mean "has next
sibling", while SwitchField receives isGroupParent={hasVisibleChildren} to mean
"has visible children"; unify and clarify this by either renaming the prop(s)
and updating call sites or by standardizing the semantic across components.
Specifically, decide whether the flag represents "hasVisibleChildren" (used by
SwitchField) or "hasNextSibling" (used by child FieldRenderer), then update the
SwitchField prop name and its consumers (or update FieldRenderer child prop) and
add a short inline comment/docstring in SwitchField and FieldRenderer describing
the flag's meaning so future contributors won't misuse isGroupParent.

src/components/settings/fields.tsx (1)

1-37: Consider a constrained icon map to improve maintainability and reduce tree-shaking uncertainty.

The dynamic icon lookup pattern (LucideIcons as any)[element.icon] relies on property access at runtime. While lucide-react 0.563.0 supports tree-shaking via ES modules, modern bundlers may struggle to optimize dynamic property access, potentially including unused icons in the bundle. A constrained map (e.g., { [key: string]: React.ComponentType }) would make available icons explicit and help bundlers optimize more predictably. This is especially important if the component is used in performance-sensitive contexts or shipped to high-latency networks.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/fields.tsx` around lines 1 - 37, Replace the runtime
dynamic lookup (currently using (LucideIcons as any)[element.icon]) with an
explicit, constrained icon map: define a const ICON_MAP: { [key: string]:
React.ComponentType } listing only the icons you actually use (e.g., FileText,
Info, Eye, EyeOff, ArrowUpRight, RefreshCcw, CircleCheck) and then look up icons
via ICON_MAP[element.icon] with a fallback component; update usages in this file
(where LucideIcons and element.icon are referenced) to import/use ICON_MAP
instead of dynamic property access so bundlers can statically analyze and
tree-shake unused icons.

🤖 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/settings/fields.tsx`:
- Around line 191-218: The ShowHideField (and likewise ColorPickerField and the
GA connected header component) currently destructure only { element, onChange }
and fail to accept/forward rest props, so nested/group flags like
isNested/isGroupParent aren’t applied; update the component signatures to accept
a rest spread (e.g., { element, onChange, ...rest }: FieldComponentProps) and
pass ...rest into the FieldWrapper call (FieldWrapper element={element}
layout={...} {...rest}) so the wrapper receives nested/group styling flags and
other forwarded props.
- Around line 303-394: Replace the inline HTML string used for the FieldWrapper
description in GoogleAnalyticsField with proper JSX (i.e., render the success
message and the Disconnect <a href={value?.disconnect_url || "#"}>…</a> link as
React nodes instead of a concatenated string) so attributes and URLs aren’t
injected as raw HTML; update the element passed to FieldWrapper (the object
spread around element with icon/description) to use a React fragment/JSX for
description. Wire up the Refresh Button (RefreshCcw) by adding an onClick
handler on that Button to call a re-fetch function (implement a fetchProfiles or
refreshProfiles function and call onClick={() => refreshProfiles()}) or
disable/remove the Button if refresh isn’t implemented to avoid a dead control.
Also replace the Wikipedia hotlink for the Google "G" image with a locally
hosted asset import (or reference a project static asset) and use that imported
asset in the img src to avoid external hotlinking.
- Around line 872-917: CombineInputField currently sets const value =
(element.value as any) || {} which treats non-object truthy values as objects;
change this to ensure value is a plain object (e.g., check typeof element.value
=== "object" && element.value !== null && !Array.isArray(element.value) and
fallback to {} otherwise) so Object.keys(value) and subsequent lookups for
percentageKey/numberKey are safe; update uses in CombineInput,
onPercentageChange and onNumberChange to rely on this guarded value variable.
- Around line 119-165: The TooltipTrigger currently renders a button by default
and you nest another explicit <button> inside (causing invalid nested buttons)
in FieldLabel, GoogleAnalyticsField, and the settings-content section headers;
fix each occurrence by removing the inner <button> and instead use
TooltipTrigger's render prop to render your custom button (with the Info icon,
className, and type="button") so the trigger becomes the single button element,
or alternatively let TooltipTrigger render the default and move the Info icon
directly into its child via render—update TooltipTrigger usages in FieldLabel,
GoogleAnalyticsField, and the settings-content header blocks to use the render
prop and eliminate nested <button> elements.

In `@src/components/settings/settings-types.ts`:
- Around line 38-42: The SettingsElement type currently requires is_danger which
is a breaking change; update SettingsElement to make is_danger optional
(is_danger?: boolean) and update all render sites to treat missing values as
false — specifically check places referencing SettingsElement such as the
SettingsContent renderer (settings-content.tsx access of is_danger),
settings-formatter, and Settings.stories where objects are spread, and ensure
they use a fallback (e.g., const dangerous = element.is_danger ?? false or
equivalent) so older schemas that omit is_danger continue to render safely.

In `@src/components/ui/copy-input.tsx`:
- Around line 19-27: The handleCopy function should await
navigator.clipboard.writeText and handle failures: wrap the writeText call in
try/catch, only call setCopied(true) and onCopy(value) on success, and log or
surface errors otherwise; also store the timeout id in a ref (e.g.,
copyTimeoutRef) and clearTimeout(copyTimeoutRef.current) before creating a new
timeout and in a useEffect cleanup to prevent stale timers updating state after
unmount (or use an isMounted ref to guard setCopied calls). Update the
handleCopy logic and add the cleanup effect to the component to ensure safe
async clipboard handling and timer cleanup.

In `@src/components/ui/radio-group.tsx`:
- Around line 162-173: The build fails because RawHTML expects string children
but label is a React.ReactNode; update the JSX around FieldTitle/RawHTML so you
only render <RawHTML>{label}</RawHTML> when typeof label === 'string', and
otherwise render the label directly (e.g., <FieldTitle
className="font-bold">{label}</FieldTitle> or wrap non-string label without
RawHTML); apply the same guard where RawHTML is used again (the other occurrence
around line 228) to ensure RawHTML always receives a string.

---

Outside diff comments:
In `@src/components/settings/fields.tsx`:
- Around line 43-117: FieldWrapper currently renders element.validationError via
the RawHTML component which allows untrusted HTML injection; change both
occurrences that wrap <RawHTML>{element.validationError}</RawHTML> to render
plain text (e.g., a simple div/span with the error string) or ensure the error
is sanitized before insertion so no HTML is interpreted. Update the two places
in FieldWrapper (the full-width branch and the grid branch) to stop using
RawHTML and instead render element.validationError as escaped/plain text, and
ensure any tests or callers (e.g., FieldRenderer) still provide a string rather
than HTML.
- Around line 520-645: The LabeledCheckbox component is being passed an
unsupported image prop; remove image={option.image} from both MulticheckField
and PreviewMulticheckField LabeledCheckbox callsites (references:
MulticheckField, PreviewMulticheckField, and LabeledCheckbox) so only supported
props (checked, onCheckedChange, label, className, description, etc.) are
passed, or alternatively add image support to LabeledCheckboxProps and its
implementation if the design requires images.

In `@src/components/settings/index.tsx`:
- Around line 116-134: The title prop is being rendered via RawHTML (see RawHTML
usage around the title occurrences in this Settings component), creating an XSS
risk; either sanitize title before passing it to RawHTML (e.g., run it through a
trusted sanitizer like DOMPurify in the component or a helper and then pass the
sanitized string to RawHTML) or stop using RawHTML and render the title as plain
text (replace <RawHTML>{title}</RawHTML> with a text-only render) in both mobile
and desktop title locations; apply the same change for other RawHTML usages in
this component (descriptions, labels, validation errors) or clearly document
that callers must pass already-sanitized HTML.

In `@src/components/settings/settings-content.tsx`:
- Around line 292-327: The variable allChildrenAreFields in SettingsSubSection
can be undefined when element.children is absent; change its initialization to a
strict boolean by using nullish coalescing (e.g., set allChildrenAreFields =
element.children?.every(...) ?? false) so downstream className decisions and
conditional rendering use a clear true/false value; update the declaration in
the SettingsSubSection function where allChildrenAreFields is defined.
- Around line 51-143: The doc-link anchor (contentSource.doc_link and
contentSource.doc_link_text) can be icon-only which is inaccessible, and the
component violates its JSDoc by returning null when renderSaveButton is absent
though onSave (handleSave) may exist; update the anchor to always include an
accessible label by using contentSource.doc_link_text ?? 'Docs' and add an
aria-label (e.g., aria-label={contentSource.doc_link_text ?? 'Docs'}) alongside
the FileText icon, and implement the documented default save control: when
showSaveArea is true and renderSaveButton is not provided, render a default
icon-only save button that calls handleSave (and reflects dirty state) so
consumers get the advertised fallback, or alternatively update the JSDoc if you
prefer requiring renderSaveButton (but do one or the other to restore contract
consistency.

In `@src/components/ui/rich-text-editor.tsx`:
- Around line 185-246: Remove the duplicated Link toolbar button: locate the
second <button> with title="Link" that calls executeCommand("createLink", url)
(the one appearing after the second divider) and delete that entire button block
so only the original Link button remains; ensure you keep the first Link button
(the one before the Quote/Lists/Align group) and that no other handlers or
imports (e.g., Link icon) are removed.

In `@src/components/ui/scroll-area.tsx`:
- Around line 10-25: The current ScrollArea component spreads all props into
ScrollAreaPrimitive.Root which leaks the custom scrollBarClassName prop;
destructure scrollBarClassName out of the incoming props (e.g., function
ScrollArea({ className, children, scrollBarClassName, ...props }:
ScrollAreaRootType)) and then spread only the remaining props into
ScrollAreaPrimitive.Root while passing scrollBarClassName explicitly to the
ScrollBar component to prevent an invalid DOM attribute from being forwarded.

---

Nitpick comments:
In `@src/components/settings/field-renderer.tsx`:
- Around line 68-90: The usages of the isGroupParent prop are inconsistent:
FieldRenderer passes isGroupParent={index < length - 1} to child FieldRenderer
to mean "has next sibling", while SwitchField receives
isGroupParent={hasVisibleChildren} to mean "has visible children"; unify and
clarify this by either renaming the prop(s) and updating call sites or by
standardizing the semantic across components. Specifically, decide whether the
flag represents "hasVisibleChildren" (used by SwitchField) or "hasNextSibling"
(used by child FieldRenderer), then update the SwitchField prop name and its
consumers (or update FieldRenderer child prop) and add a short inline
comment/docstring in SwitchField and FieldRenderer describing the flag's meaning
so future contributors won't misuse isGroupParent.

In `@src/components/settings/fields.tsx`:
- Around line 1-37: Replace the runtime dynamic lookup (currently using
(LucideIcons as any)[element.icon]) with an explicit, constrained icon map:
define a const ICON_MAP: { [key: string]: React.ComponentType } listing only the
icons you actually use (e.g., FileText, Info, Eye, EyeOff, ArrowUpRight,
RefreshCcw, CircleCheck) and then look up icons via ICON_MAP[element.icon] with
a fallback component; update usages in this file (where LucideIcons and
element.icon are referenced) to import/use ICON_MAP instead of dynamic property
access so bundlers can statically analyze and tree-shake unused icons.

In `@src/components/settings/settings-types.ts`:
- Around line 177-182: Add short doc comments to the FieldComponentProps
interface describing the semantics and propagation rules for isNested and
isGroupParent: explain that isNested indicates the field is rendered inside
another field/group (set by parent field renderers when embedding children), and
isGroupParent signals that this field acts as a container for child fields (set
true on group/container components so children can detect group context); attach
these comments directly above the isNested and isGroupParent properties in
FieldComponentProps and mention expected propagation behavior by parent
components and child consumers, referencing SettingsElement usage where
relevant.

In `@src/components/ui/RadioGroup.stories.tsx`:
- Around line 386-453: The Image Card and Icon Card stories (ImageCard,
IconCard) use external picsum URLs inside the Demo functions passed to
RadioImageCard and RadioIconCard which makes Storybook flaky; replace those
external image/icon URLs with local static assets or embedded data URIs and add
simple local fixtures (e.g., import ./fixtures/image1.png, image2.png or use
small base64 data URIs) and update the RadioImageCard and RadioIconCard props to
use those local sources so the stories are deterministic in CI/offline.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Find TooltipTrigger import and implementation
rg -n "TooltipTrigger" src/components/settings/fields.tsx -A 2 -B 2

Repository: getdokan/plugin-ui

Length of output: 1096

---

🏁 Script executed:

# Check if TooltipTrigger is imported and from where
rg -n "import.*TooltipTrigger|from.*Tooltip" src/components/settings/fields.tsx

Repository: getdokan/plugin-ui

Length of output: 44

---

🏁 Script executed:

# Find TooltipTrigger in the UI components (likely shadcn)
fd -e tsx -e ts . src/components/ui 2>/dev/null | head -20

Repository: getdokan/plugin-ui

Length of output: 830

---

🏁 Script executed:

# Search for Tooltip implementation in the codebase
rg -n "export.*Tooltip" src/components/ui

Repository: getdokan/plugin-ui

Length of output: 278

---

🏁 Script executed:

# Read the tooltip.tsx file to understand TooltipTrigger implementation
cat -n src/components/ui/tooltip.tsx

Repository: getdokan/plugin-ui

Length of output: 3724

---

🏁 Script executed:

# Also check if there are any other instances of TooltipTrigger in the codebase
rg -n "TooltipTrigger" --type tsx --type ts -A 3 | head -60

Repository: getdokan/plugin-ui

Length of output: 89

---

🏁 Script executed:

# Search for all TooltipTrigger usages to see patterns
rg -n "TooltipTrigger" --type ts --type tsx -A 4 -B 1

Repository: getdokan/plugin-ui

Length of output: 89

---

🏁 Script executed:

# Check package.json to see `@base-ui` version
cat package.json | grep -A 10 -B 10 "@base-ui"

Repository: getdokan/plugin-ui

Length of output: 638

---

🏁 Script executed:

# Search for asChild usage in the codebase
rg -n "asChild" --type ts --type tsx

Repository: getdokan/plugin-ui

Length of output: 89

---

🏁 Script executed:

# Search for all TooltipTrigger usages without type filter
rg -n "TooltipTrigger" -A 4 -B 1

Repository: getdokan/plugin-ui

Length of output: 12457

---

🏁 Script executed:

# Search for asChild usage without type filter
rg -n "asChild"

Repository: getdokan/plugin-ui

Length of output: 1119

---

🏁 Script executed:

# Check `@base-ui` documentation via web search for how Trigger renders
web_search
`@base-ui` react tooltip trigger component default element type

Repository: getdokan/plugin-ui

Length of output: 159

---

🌐 Web query:

@base-ui react tooltip trigger component renders button children

💡 Result:

In Base UI (@base-ui/react/tooltip), <Tooltip.Trigger> renders a <button> by default, and any children you pass become the button’s contents (e.g. <Tooltip.Trigger>Button</Tooltip.Trigger>). [1]

If you don’t want it to render its own <button> (for example, you already have a button component), compose/replace the trigger element using the render prop (your component must forward the ref and spread the received props onto the underlying DOM element). [1][2]

Example patterns from the docs:

• render={<Toolbar.Button />} when using Tooltip inside a Toolbar. [2]
• render={<MyButton />} for a custom button component. [2]

---

Use render prop instead of asChild to fix nested <button> elements (invalid DOM/a11y).

@base-ui/react/tooltip's Trigger renders a <button> by default, so wrapping an explicit <button> inside creates invalid nested buttons. Either remove the inner button or use the render prop to replace the default button:

Example fix pattern

- <TooltipTrigger>
-   <button type="button" className="inline-flex">
-     <Info className="size-3.5 text-muted-foreground cursor-help" />
-   </button>
- </TooltipTrigger>
+ <TooltipTrigger render={<button type="button" className="inline-flex" />}>
+   <Info className="size-3.5 text-muted-foreground cursor-help" />
+ </TooltipTrigger>

Or simply remove the button and let Trigger handle it:

- <TooltipTrigger>
-   <button type="button" className="inline-flex">
-     <Info className="size-3.5 text-muted-foreground cursor-help" />
-   </button>
- </TooltipTrigger>
+ <TooltipTrigger>
+   <Info className="size-3.5 text-muted-foreground cursor-help" />
+ </TooltipTrigger>

Affects:

• FieldLabel (lines 145–149)
• GoogleAnalyticsField (lines 353–357)
• src/components/settings/settings-content.tsx section headers (lines 205–209)

📝 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

	function FieldLabel({ element }: { element: SettingsElement }) {
	const displayLabel = element.label || element.title || '';
	const IconComponent = element.icon ? (LucideIcons as any)[element.icon] : null;
	return (
	<div className="flex flex-col gap-1 w-full">
	<div className="flex items-center gap-2">
	{element.image_url && (
	<img
	src={element.image_url}
	alt=""
	className="w-5 h-5 object-contain"
	/>
	)}
	<span className="text-sm font-semibold text-foreground">
	{displayLabel}
	</span>
	{element.tooltip && (
	<TooltipProvider>
	<Tooltip>
	<TooltipTrigger>
	<button type="button" className="inline-flex">
	<Info className="size-3.5 text-muted-foreground cursor-help" />
	</button>
	</TooltipTrigger>
	<TooltipContent>
	<p className="max-w-xs text-xs">{element.tooltip}</p>
	</TooltipContent>
	</Tooltip>
	</TooltipProvider>
	<div className="flex items-start gap-3">
	{element.image_url && (
	<img
	src={element.image_url}
	alt=""
	className="w-11 h-11 object-contain border border-border rounded-md p-2"
	/>
	)}
	<div className="flex flex-col gap-1 w-full">
	<div className="flex items-center gap-2">
	<span className={ cn( 'text-sm font-semibold', element?.is_danger ? 'text-destructive' : 'text-foreground' ) }>
	{displayLabel}
	</span>
	{IconComponent && (
	<IconComponent className={ cn( 'size-4', element?.is_danger ? 'text-destructive' : 'text-primary' ) } />
	)}
	{element.tooltip && (
	<TooltipProvider>
	<Tooltip>
	<TooltipTrigger>
	<button type="button" className="inline-flex">
	<Info className={ cn( 'size-3.5 cursor-help', element?.is_danger ? 'text-destructive' : 'text-muted-foreground' ) } />
	</button>
	</TooltipTrigger>
	<TooltipContent>
	<p className="max-w-xs text-xs">{element.tooltip}</p>
	</TooltipContent>
	</Tooltip>
	</TooltipProvider>
	)}
	</div>
	{element.description && (
	<div className={ cn( 'text-xs leading-relaxed', element?.is_danger ? 'text-destructive' : 'text-muted-foreground' ) }>
	<RawHTML>{element.description}</RawHTML>
	</div>
	)}
	</div>
	{element.description && (
	<p className="text-xs text-muted-foreground leading-relaxed">
	{element.description}
	</p>
	)}
	</div>
	);
	}
	function FieldLabel({ element }: { element: SettingsElement }) {
	const displayLabel = element.label || element.title || '';
	const IconComponent = element.icon ? (LucideIcons as any)[element.icon] : null;
	return (
	<div className="flex items-start gap-3">
	{element.image_url && (
	<img
	src={element.image_url}
	alt=""
	className="w-11 h-11 object-contain border border-border rounded-md p-2"
	/>
	)}
	<div className="flex flex-col gap-1 w-full">
	<div className="flex items-center gap-2">
	<span className={ cn( 'text-sm font-semibold', element?.is_danger ? 'text-destructive' : 'text-foreground' ) }>
	{displayLabel}
	</span>
	{IconComponent && (
	<IconComponent className={ cn( 'size-4', element?.is_danger ? 'text-destructive' : 'text-primary' ) } />
	)}
	{element.tooltip && (
	<TooltipProvider>
	<Tooltip>
	<TooltipTrigger>
	<Info className={ cn( 'size-3.5 cursor-help', element?.is_danger ? 'text-destructive' : 'text-muted-foreground' ) } />
	</TooltipTrigger>
	<TooltipContent>
	<p className="max-w-xs text-xs">{element.tooltip}</p>
	</TooltipContent>
	</Tooltip>
	</TooltipProvider>
	)}
	</div>
	{element.description && (
	<div className={ cn( 'text-xs leading-relaxed', element?.is_danger ? 'text-destructive' : 'text-muted-foreground' ) }>
	<RawHTML>{element.description}</RawHTML>
	</div>
	)}
	</div>
	</div>
	);
	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/fields.tsx` around lines 119 - 165, The
TooltipTrigger currently renders a button by default and you nest another
explicit <button> inside (causing invalid nested buttons) in FieldLabel,
GoogleAnalyticsField, and the settings-content section headers; fix each
occurrence by removing the inner <button> and instead use TooltipTrigger's
render prop to render your custom button (with the Info icon, className, and
type="button") so the trigger becomes the single button element, or
alternatively let TooltipTrigger render the default and move the Info icon
directly into its child via render—update TooltipTrigger usages in FieldLabel,
GoogleAnalyticsField, and the settings-content header blocks to use the render
prop and eliminate nested <button> elements.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

ShowHideField / ColorPickerField (and GA “connected” header) drop ...rest, so nested/group styling won’t apply.

These components should accept ...rest and forward it to FieldWrapper (same as the other fields), otherwise isNested / isGroupParent will be ignored for these variants.

Proposed fixes

-export function ShowHideField({ element, onChange }: FieldComponentProps) {
+export function ShowHideField({ element, onChange, ...rest }: FieldComponentProps) {
   const [show, setShow] = useState(false);

   return (
-    <FieldWrapper element={element} layout={ element.layout ?? "horizontal" }>
+    <FieldWrapper element={element} layout={element.layout ?? "horizontal"} {...rest}>
       <div className="relative max-w-56 md:max-w-full w-full">
         <Input
           type={show ? "text" : "password"}

-export function ColorPickerField({ element, onChange }: FieldComponentProps) {
+export function ColorPickerField({ element, onChange, ...rest }: FieldComponentProps) {
   const value = String(element.value ?? element.default ?? "#000000");

   return (
-    <FieldWrapper element={element} layout={element.layout ?? "horizontal"}>
+    <FieldWrapper element={element} layout={element.layout ?? "horizontal"} {...rest}>
       <ColorPicker
         value={value}

-      <FieldWrapper
+      <FieldWrapper
         element={{
           ...element,
           icon: "CircleCheck",
           description: `Successfully connected ...`,
         }}
+        {...rest}
       >
         <div />
       </FieldWrapper>

Also applies to: 438-451, 303-344

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/fields.tsx` around lines 191 - 218, The ShowHideField
(and likewise ColorPickerField and the GA connected header component) currently
destructure only { element, onChange } and fail to accept/forward rest props, so
nested/group flags like isNested/isGroupParent aren’t applied; update the
component signatures to accept a rest spread (e.g., { element, onChange, ...rest
}: FieldComponentProps) and pass ...rest into the FieldWrapper call
(FieldWrapper element={element} layout={...} {...rest}) so the wrapper receives
nested/group styling flags and other forwarded props.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

GoogleAnalyticsField: avoid HTML string-building for links + implement/omit the “Refresh” action.

• Building a long HTML string with interpolated URLs (disconnect_url) is hard to maintain and easy to misuse; prefer JSX (<a href={...}>…</a>) so URLs aren’t injected into raw HTML.
• The “Refresh” button currently has no onClick; either wire it (e.g., re-fetch profiles) or remove/disable it to avoid a dead control.
• Consider hosting the Google “G” asset locally (hotlinking to Wikipedia can be brittle).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/fields.tsx` around lines 303 - 394, Replace the
inline HTML string used for the FieldWrapper description in GoogleAnalyticsField
with proper JSX (i.e., render the success message and the Disconnect <a
href={value?.disconnect_url || "#"}>…</a> link as React nodes instead of a
concatenated string) so attributes and URLs aren’t injected as raw HTML; update
the element passed to FieldWrapper (the object spread around element with
icon/description) to use a React fragment/JSX for description. Wire up the
Refresh Button (RefreshCcw) by adding an onClick handler on that Button to call
a re-fetch function (implement a fetchProfiles or refreshProfiles function and
call onClick={() => refreshProfiles()}) or disable/remove the Button if refresh
isn’t implemented to avoid a dead control. Also replace the Wikipedia hotlink
for the Google "G" image with a locally hosted asset import (or reference a
project static asset) and use that imported asset in the img src to avoid
external hotlinking.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

CombineInputField: ensure value is a plain object before Object.keys() and key lookup.

Right now const value = (element.value as any) || {} will treat non-object truthy values (e.g., strings) as valid and produce odd keys. Safer guard avoids surprising behavior if schema data is malformed.

Proposed guard

-export function CombineInputField({ element, onChange, ...rest }: FieldComponentProps) {
-  const value = (element.value as any) || {};
+export function CombineInputField({ element, onChange, ...rest }: FieldComponentProps) {
+  const value =
+    typeof element.value === "object" && element.value !== null ? (element.value as any) : {};

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/fields.tsx` around lines 872 - 917, CombineInputField
currently sets const value = (element.value as any) || {} which treats
non-object truthy values as objects; change this to ensure value is a plain
object (e.g., check typeof element.value === "object" && element.value !== null
&& !Array.isArray(element.value) and fallback to {} otherwise) so
Object.keys(value) and subsequent lookups for percentageKey/numberKey are safe;
update uses in CombineInput, onPercentageChange and onNumberChange to rely on
this guarded value variable.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Look for schema construction sites and SettingsElement usage
echo "=== Searching for schema construction patterns ==="
rg -n --type=ts --type=tsx "schema\s*:\s*\[" . | head -20

echo ""
echo "=== Searching for SettingsElement usage ==="
rg -n --type=ts --type=tsx "\bSettingsElement\b" .

echo ""
echo "=== Searching for is_danger usage ==="
rg -n "is_danger" .

Repository: getdokan/plugin-ui

Length of output: 157

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Look for schema construction and SettingsElement usage
echo "=== Searching for SettingsElement usage ==="
rg -n "SettingsElement" . --type ts

echo ""
echo "=== Searching for is_danger usage ==="
rg -n "is_danger" .

echo ""
echo "=== Searching for schema construction patterns ==="
rg -n "schema\s*:\s*\[" . -A 5 | head -50

Repository: getdokan/plugin-ui

Length of output: 8484

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Checking settings-formatter.ts enriched object construction ==="
sed -n '51,100p' ./src/components/settings/settings-formatter.ts

echo ""
echo "=== Checking field-renderer.tsx mergedElement construction ==="
sed -n '50,75p' ./src/components/settings/field-renderer.tsx

echo ""
echo "=== Looking for schema construction in Settings.stories.tsx (sample check) ==="
sed -n '97,150p' ./src/components/settings/Settings.stories.tsx

Repository: getdokan/plugin-ui

Length of output: 5010

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Checking for is_danger presence in all story schema objects ==="
rg -c "is_danger" ./src/components/settings/Settings.stories.tsx

echo ""
echo "=== Counting total schema object declarations in stories ==="
rg -c "type: '" ./src/components/settings/Settings.stories.tsx | head -1

echo ""
echo "=== Checking specific schema definitions for is_danger ==="
grep -n "is_danger" ./src/components/settings/Settings.stories.tsx

echo ""
echo "=== Looking at one complete schema object example ==="
sed -n '544,650p' ./src/components/settings/Settings.stories.tsx

Repository: getdokan/plugin-ui

Length of output: 3361

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Checking settings-content.tsx line 191 context ==="
sed -n '185,200p' ./src/components/settings/settings-content.tsx

echo ""
echo "=== Verifying if stories compile with current strict types ==="
rg -B 5 "const sampleSchema:" ./src/components/settings/Settings.stories.tsx | head -20

Repository: getdokan/plugin-ui

Length of output: 1371

---

is_danger: boolean being required is a breaking type change.

Story schemas and constructor sites don't consistently provide is_danger (only 2 out of ~97 schema examples include it). Settings.stories.tsx and settings-formatter.ts omit it when spreading objects, yet settings-content.tsx line 191 accesses it without optional chaining. Make it optional and treat missing as false in renderers to maintain backward compatibility.

Proposed type change

 export type SettingsElement = {
     id: string;
     type: 'page' | 'subpage' | 'tab' | 'section' | 'subsection' | 'field' | 'fieldgroup' | string;
-    is_danger: boolean;
+    is_danger?: boolean;
     variant?: string;

📝 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

	export type SettingsElement = {
	id: string;
	type: 'page' | 'subpage' | 'tab' | 'section' | 'subsection' | 'field' | 'fieldgroup' | string;
	is_danger: boolean;
	variant?: string;
	export type SettingsElement = {
	id: string;
	type: 'page' | 'subpage' | 'tab' | 'section' | 'subsection' | 'field' | 'fieldgroup' | string;
	is_danger?: boolean;
	variant?: string;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/settings-types.ts` around lines 38 - 42, The
SettingsElement type currently requires is_danger which is a breaking change;
update SettingsElement to make is_danger optional (is_danger?: boolean) and
update all render sites to treat missing values as false — specifically check
places referencing SettingsElement such as the SettingsContent renderer
(settings-content.tsx access of is_danger), settings-formatter, and
Settings.stories where objects are spread, and ensure they use a fallback (e.g.,
const dangerous = element.is_danger ?? false or equivalent) so older schemas
that omit is_danger continue to render safely.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Handle clipboard errors and clear pending timeouts.
navigator.clipboard.writeText is async and can fail (permissions/HTTP). Also, repeated clicks or unmounts can leave stale timers that update state after unmount.

🛠️ Suggested fix

-import React, { useState } from "react";
+import React, { useEffect, useRef, useState } from "react";
@@
-  const [copied, setCopied] = useState(false);
+  const [copied, setCopied] = useState(false);
+  const timeoutRef = useRef<number | null>(null);
+
+  useEffect(() => {
+    return () => {
+      if (timeoutRef.current) window.clearTimeout(timeoutRef.current);
+    };
+  }, []);
@@
-  const handleCopy = () => {
+  const handleCopy = async () => {
     if (!value) return;
-    navigator.clipboard.writeText(value);
-    setCopied(true);
-    if (onCopy) onCopy(value);
-    setTimeout(() => setCopied(false), successDuration);
+    try {
+      await navigator.clipboard.writeText(value);
+      setCopied(true);
+      if (onCopy) onCopy(value);
+      if (timeoutRef.current) window.clearTimeout(timeoutRef.current);
+      timeoutRef.current = window.setTimeout(
+        () => setCopied(false),
+        successDuration
+      );
+    } catch {
+      // Optionally surface an error state or noop
+    }
   };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/ui/copy-input.tsx` around lines 19 - 27, The handleCopy
function should await navigator.clipboard.writeText and handle failures: wrap
the writeText call in try/catch, only call setCopied(true) and onCopy(value) on
success, and log or surface errors otherwise; also store the timeout id in a ref
(e.g., copyTimeoutRef) and clearTimeout(copyTimeoutRef.current) before creating
a new timeout and in a useEffect cleanup to prevent stale timers updating state
after unmount (or use an isMounted ref to guard setCopied calls). Update the
handleCopy logic and add the cleanup effect to the component to ensure safe
async clipboard handling and timer cleanup.

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (1)

src/components/settings/fields.tsx (1)

3-4: Redundant double import from lucide-react.

All named icons (FileText, Info, Eye, EyeOff, ArrowUpRight, RefreshCcw, CircleCheck) are already available via the LucideIcons namespace import on line 3. The separate named import on line 4 is redundant.

♻️ Suggested fix

  import * as LucideIcons from "lucide-react";
- import { FileText, Info, Eye, EyeOff, ArrowUpRight, RefreshCcw, CircleCheck } from "lucide-react";

Then reference them as LucideIcons.FileText, LucideIcons.Info, etc., or keep only the named import and drop the wildcard if the dynamic LucideIcons[element.icon] lookup is refactored separately.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/fields.tsx` around lines 3 - 4, There is a redundant
double import from lucide-react: remove the named import list (FileText, Info,
Eye, EyeOff, ArrowUpRight, RefreshCcw, CircleCheck) and use the existing
namespace import LucideIcons to reference icons (e.g., LucideIcons.FileText,
LucideIcons.Info, etc.), or alternatively drop the LucideIcons namespace and
keep the named imports if you prefer static references; update any usages in
this file to the chosen form so there are no duplicate imports.

🤖 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/settings/fields.tsx`:
- Around line 63-69: The merge for CSS classes in the mergedClassName
calculation uses element.className which does not exist on SettingsElement and
is therefore always undefined; update the cn(...) call in the mergedClassName
definition to remove the non-existent element.className and, if the intent was
to include the element's wrapper classes, replace it with element.wrapper_class
(or element.css_class/content_class as appropriate), so edit the mergedClassName
assignment (the cn(...) call) to use the correct SettingsElement property or
drop the dead entry.

In `@src/components/settings/settings-content.tsx`:
- Line 281: ElementRenderer passes isNested and isGroupParent into
SettingsFieldGroup but SettingsFieldGroup's prop signature currently only
accepts { element }, causing a TypeScript build error; update the
SettingsFieldGroup component (the function/component declared around the current
prop signature) to accept a props object including element, isNested?: boolean,
and isGroupParent?: boolean (or appropriate required/optional types), use those
props inside the component as needed, and ensure any internal references or
destructuring reflect the new names; also adjust any related type/interface
(prop type or Props interface) so the component and all usages compile cleanly.
- Around line 316-323: The subsection currently passes isNested to every child
via the ElementRenderer call, which causes FieldWrapper to apply !border styles
and suppress the container's divide-y borders; change the prop so isNested is
only true for the first child (e.g., pass isNested={isNested && index === 0} to
ElementRenderer) so only the first field suppresses its top border while
preserving divide-y separators for the rest.

---

Duplicate comments:
In `@src/components/settings/fields.tsx`:
- Around line 191-218: ShowHideField currently only takes { element, onChange }
and drops other FieldComponentProps like isNested / isGroupParent so
FieldWrapper loses group-context props; update the ShowHideField signature to
accept the remaining props (e.g., function ShowHideField({ element, onChange,
...rest }: FieldComponentProps) or explicitly include isNested/isGroupParent)
and pass them through to FieldWrapper (e.g., <FieldWrapper element={element}
layout={...} {...rest}>) so group/nesting flags are preserved.
- Around line 438-451: ColorPickerField currently destructures only { element,
onChange } so it drops the remaining props (such as isNested/isGroupParent) that
FieldWrapper expects; update ColorPickerField to accept and forward the rest of
FieldComponentProps (e.g., use the rest spread like ...rest) and pass them into
FieldWrapper (keep element and layout logic intact), mirroring how ShowHideField
forwards props so isNested/isGroupParent and other flags are preserved.
- Around line 872-873: The CombineInputField currently does const value =
(element.value as any) || {} which treats any truthy non-object (string/number)
as an object and breaks subsequent Object.keys/lookup logic; change the guard to
ensure element.value is a non-null object before using it (e.g., check typeof
element.value === "object" && element.value !== null or use an isPlainObject
helper) and otherwise default to an empty object so that Object.keys(value) and
value[percentageKey]/value[numberKey] behave safely; update the value typing to
Record<string, any> or a proper interface to reflect the object shape used by
percentageKey/numberKey.
- Around line 335-343: The description currently builds raw HTML with an
interpolated value?.disconnect_url which is an XSS risk; update the FieldWrapper
usage in GoogleAnalyticsField (and the element.description prop) to pass a React
node instead of a raw HTML string: build the text and the anchor as JSX (e.g.,
plain text + <a href={safeDisconnectUrl} ...>Disconnect</a>) and render via
FieldLabel/RawHTML as normal text rather than injecting HTML. Ensure you
normalize/validate the disconnect URL (fallback to "#" if invalid) and add safe
anchor attributes (e.g., rel="noopener noreferrer", optional target) so the link
cannot be exploited.
- Around line 145-154: The TooltipTrigger currently renders a <button> by
default, so the inner <button type="button"> creates invalid nested buttons; fix
both occurrences (TooltipTrigger usage in this file and in GoogleAnalyticsField)
by either removing the inner button and passing the Info icon directly as the
TooltipTrigger child while moving the className/props onto TooltipTrigger, or by
using TooltipTrigger's asChild prop and keep the inner <button> (so the trigger
will render the provided child instead of its own button). Update the
TooltipTrigger usages (and corresponding className/element?.is_danger logic)
accordingly to avoid nested buttons.

In `@src/components/settings/settings-content.tsx`:
- Around line 219-229: The TooltipTrigger currently wraps a native <button>,
causing nested interactive elements; fix by using TooltipTrigger's asChild prop
so it forwards to your existing <button> (e.g., change <TooltipTrigger> to
<TooltipTrigger asChild> and keep the inner <button> with its classes and the
<Info /> icon), or alternatively replace the inner <button> with a non-button
element (e.g., <span role="button" tabIndex={0}>) while keeping TooltipTrigger
as the outer wrapper; update the TooltipTrigger/Info/TooltipContent usage
accordingly and ensure the tooltip string (tooltip) remains unchanged.

In `@src/components/settings/settings-types.ts`:
- Line 41: The property is_danger is currently required, which breaks existing
schemas; make it optional by changing its declaration to an optional property
(is_danger?) in the settings type definition so existing code that omits it
won't fail; update the type in src/components/settings/settings-types.ts where
is_danger is declared to use the optional modifier and run type checks to ensure
no callers assume it's always present.

---

Nitpick comments:
In `@src/components/settings/fields.tsx`:
- Around line 3-4: There is a redundant double import from lucide-react: remove
the named import list (FileText, Info, Eye, EyeOff, ArrowUpRight, RefreshCcw,
CircleCheck) and use the existing namespace import LucideIcons to reference
icons (e.g., LucideIcons.FileText, LucideIcons.Info, etc.), or alternatively
drop the LucideIcons namespace and keep the named imports if you prefer static
references; update any usages in this file to the chosen form so there are no
duplicate imports.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

element.className is not a property of SettingsElement — always undefined.

SettingsElement defines css_class, wrapper_class, and content_class, but not className. This line is dead code. Consider removing it or replacing with element.wrapper_class if that's the intent.

🔧 Suggested fix

  const mergedClassName = cn(
    className,
-   element.className,
    element.css_class,
    isGroupParent && "!pb-2",
    isNested && "!pt-2 !border-t-0 !border-none",
  );

📝 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

	const mergedClassName = cn(
	className,
	element.className,
	element.css_class,
	isGroupParent && "!pb-2",
	isNested && "!pt-2 !border-t-0 !border-none",
	);
	const mergedClassName = cn(
	className,
	element.css_class,
	isGroupParent && "!pb-2",
	isNested && "!pt-2 !border-t-0 !border-none",
	);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/fields.tsx` around lines 63 - 69, The merge for CSS
classes in the mergedClassName calculation uses element.className which does not
exist on SettingsElement and is therefore always undefined; update the cn(...)
call in the mergedClassName definition to remove the non-existent
element.className and, if the intent was to include the element's wrapper
classes, replace it with element.wrapper_class (or
element.css_class/content_class as appropriate), so edit the mergedClassName
assignment (the cn(...) call) to use the correct SettingsElement property or
drop the dead entry.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

isNested propagated to all subsection children may suppress divide-y borders.

Line 320 passes isNested={isNested} to every child of the subsection. When isNested=true, FieldWrapper applies !border-t-0 !border-none (Tailwind !important), which would override the divide-y divide-border borders set on the children container (line 315), removing all intra-subsection field separators.

If the intent is to only suppress the top border of the first child (to connect the subsection header to its first field), apply isNested only when index === 0:

🔧 Suggested change

-isNested={isNested}
+isNested={isNested && index === 0}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/settings-content.tsx` around lines 316 - 323, The
subsection currently passes isNested to every child via the ElementRenderer
call, which causes FieldWrapper to apply !border styles and suppress the
container's divide-y borders; change the prop so isNested is only true for the
first child (e.g., pass isNested={isNested && index === 0} to ElementRenderer)
so only the first field suppresses its top border while preserving divide-y
separators for the rest.

[coderabbitai]

Actionable comments posted: 10

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/components/ui/checkbox.tsx (1)

99-107: ⚠️ Potential issue | 🟠 Major

image prop leaks to the DOM in CheckboxCard.

CheckboxCardProps extends LabeledCheckboxProps, so image is now part of the accepted prop surface. Because CheckboxCard does not explicitly destructure image, it falls into ...props and is forwarded all the way to CheckboxPrimitive.Root's underlying DOM element, producing a React unknown-DOM-attribute warning (and potentially invalid HTML).

🛠️ Proposed fix – sink `image` before spread

 function CheckboxCard({ 
   label, 
   description, 
+  image: _image,   // accepted but intentionally unused in this variant
   className,
   orientation = "horizontal",
   position = "left",
   disabled,
   ...props 
 }: CheckboxCardProps) {

Alternatively, exclude image from CheckboxCardProps entirely if the card variant is not intended to support images:

-interface CheckboxCardProps extends LabeledCheckboxProps {}
+interface CheckboxCardProps extends Omit<LabeledCheckboxProps, 'image'> {}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/ui/checkbox.tsx` around lines 99 - 107, The CheckboxCard
component accepts CheckboxCardProps (which extends LabeledCheckboxProps) and
currently forwards all remaining props via ...props so the image prop leaks onto
the DOM; update the CheckboxCard signature to explicitly destructure image
(e.g., include image in the parameter list) and do not include it in ...props so
it is "sunk" and not passed into CheckboxPrimitive.Root, or alternatively remove
image from CheckboxCardProps if card variants should not support images; ensure
you update any uses of CheckboxCard accordingly and keep CheckboxPrimitive.Root
receiving only valid DOM props.

🧹 Nitpick comments (2)

src/components/ui/checkbox.tsx (1)

65-90: Extract duplicated label/description block into a local variable.

The FieldLabel + conditional FieldDescription block is identical in both branches of the image conditional. A single local variable removes the repetition.

♻️ Proposed refactor

+  const labelContent = (
+    <>
+      <FieldLabel htmlFor={props.id}>{label}</FieldLabel>
+      {description && <FieldDescription>{description}</FieldDescription>}
+    </>
+  );
+
   <FieldContent>
     {image ? (
       <div className="flex items-center gap-3">
         <img src={image} alt="" className="size-10 rounded object-cover shrink-0" />
-        <div className="flex flex-col gap-1">
-          <FieldLabel htmlFor={props.id}>
-            {label}
-          </FieldLabel>
-          {description && (
-            <FieldDescription>
-              {description}
-            </FieldDescription>
-          )}
-        </div>
+        <div className="flex flex-col gap-1">{labelContent}</div>
       </div>
     ) : (
-      <>
-        <FieldLabel htmlFor={props.id}>
-          {label}
-        </FieldLabel>
-        {description && (
-          <FieldDescription>
-            {description}
-          </FieldDescription>
-        )}
-      </>
+      labelContent
     )}
   </FieldContent>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/ui/checkbox.tsx` around lines 65 - 90, Extract the duplicated
FieldLabel + conditional FieldDescription JSX into a single local variable (e.g.
const labelBlock) inside the Checkbox component in
src/components/ui/checkbox.tsx and use that variable in both branches of the
image conditional; ensure the extracted block still uses props.id, label and
description so the existing conditional for description remains intact and the
image branch simply renders the img plus labelBlock while the no-image branch
renders labelBlock alone.

src/components/ui/radio-group.tsx (1)

127-130: Avoid any for currentValue, image, and icon prop types.

currentValue: any, image: any, and icon: any bypass type safety. Prefer narrower types aligned with actual usage:

♻️ Proposed refactor

 interface RadioImageCardProps extends LabeledRadioProps {
-  currentValue: any;
-  image: any;
+  currentValue?: RadioPrimitive.Root.Props['value'];
+  image?: string;
 }

 interface RadioIconCardProps extends LabeledRadioProps {
-  currentValue: any;
-  icon: any;
+  icon?: string | React.ReactNode;
 }

Also applies to: 182-185

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/ui/radio-group.tsx` around lines 127 - 130, Replace the use of
`any` in the radio card props with specific React-friendly types: change
`currentValue: any` to a union such as `string | number | boolean` (or the exact
discriminant your radio uses) and change `image: any` and any `icon: any` props
to `React.ReactNode | string` (string if you accept image URLs) or a more
specific image prop type if you use Next/Image props; update the
`RadioImageCardProps` interface and the corresponding prop types in the other
component declarations (the other occurrences around the icon prop) to use these
types and import `React`/`ReactNode` as needed so you preserve type safety
across `RadioImageCardProps`, `LabeledRadioProps`, and any components that
consume `currentValue`, `image`, or `icon`.

🤖 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/settings/settings-content.tsx`:
- Around line 308-312: The use of RawHTML to render element.description is an
XSS risk if that string can ever be user-controlled; update the rendering in the
component that uses RawHTML (reference RawHTML and element.description) to
sanitize the HTML before injecting it (e.g., use a trusted sanitizer like
DOMPurify and pass the sanitized string into dangerouslySetInnerHTML) or else
document and enforce that element.description is always
developer-controlled/trusted HTML; ensure the fix replaces the RawHTML usage
with a sanitized dangerouslySetInnerHTML render or adds validation that prevents
any untrusted input from reaching element.description.
- Around line 219-229: TooltipTrigger currently renders a button and you nest
another <button> inside it (the Info button), producing invalid HTML; change
TooltipTrigger to use the asChild prop so it will forward the trigger role to
the inner element, i.e., replace TooltipTrigger with TooltipTrigger asChild and
keep the existing inner <button> (the Info icon button) as its direct child,
leaving TooltipContent, TooltipProvider and the tooltip variable usage unchanged
so the trigger is the actual button element rather than a wrapped button
component.
- Line 321: The isGroupParent prop is inverted: instead of setting it only for
the last child, make it true for every child except the last by changing the
condition on isGroupParent to check that index is less than element.children
length minus one (i.e., use an index < childrenLength - 1 comparison rather than
index === childrenLength - 1); update the prop passed where isGroupParent and
index and element.children are used so intermediate group children receive
isGroupParent=true and only the final child gets false.
- Around line 131-140: The sticky save bar is shown whenever showSaveArea is
true (currently tied to onSave) but renderSaveButton may be undefined, causing
an empty footer; update the condition or provide a fallback: change the
visibility logic so showSaveArea only becomes true when both onSave and
renderSaveButton are present (e.g., gate the rendering with renderSaveButton ||
provide a default), or implement a default save button when renderSaveButton is
undefined that calls handleSave and uses scopeId/dirty for state; make the
change around the JSX that references showSaveArea, renderSaveButton,
handleSave, scopeId and dirty so the sticky bar is only rendered when an actual
button will appear.
- Around line 74-78: The anchor that renders the FileText icon uses
contentSource.doc_link_text but falls back to an empty string, producing an
icon-only link with no accessible label; update the anchor rendering (the <a>
around FileText in settings-content.tsx) to provide an aria-label when
contentSource.doc_link_text is falsy—for example compute a label like
`${contentSource.doc_link_text ?? 'Documentation link'}` or a localized fallback
and set it on the anchor as aria-label (and keep existing visible text when
present); apply the same change to the duplicate anchor at the other occurrence
(around line 243).
- Around line 52-53: Remove the conflicting overflow class from the ScrollArea
call: in the JSX where ScrollArea is rendered (the ScrollArea element using
cn('flex flex-col overflow-y-auto', className) and
data-testid="settings-content"), stop including "overflow-y-auto" so the root
retains the component's default overflow-hidden; keep the layout classes like
"flex flex-col" and still merge incoming className, and ensure the inner
viewport (the element with h-full w-full) remains the only scroller.

In `@src/components/ui/radio-group.tsx`:
- Line 154: The data-testid attribute is currently rendered as
`settings-field-undefined` when props.id is missing; update the RadioGroup
component to only set data-testid when props.id is defined (e.g., replace
data-testid={`settings-field-${props.id}`} with a conditional such as
data-testid={props.id ? `settings-field-${props.id}` : undefined}) so the
attribute is omitted when id is absent; apply the same fix to the other
occurrence of the same expression in this file.
- Line 146: The class string passed into the cn(...) call contains the duplicate
utility "group" (i.e., "transition-colors has-data-checked:bg-transparent
dark:has-data-checked:bg-transparent p-0 group cursor-pointer group"); remove
the second "group" occurrence so the class list only includes a single "group"
token; locate the cn(...) invocation in the radio-group component (the string
shown) and update it accordingly.
- Line 6: The project imports RawHTML from `@wordpress/element` in
src/components/ui/radio-group.tsx but `@wordpress/element` is only a transitive
dependency; add "@wordpress/element" to the package.json dependencies so
consumers won't break. Update package.json's "dependencies" to include the
appropriate version (e.g., ^6.39.0 or matching your lockfile) and run npm/yarn
install to update lockfiles, then verify imports in
src/components/ui/radio-group.tsx still resolve.
- Around line 187-234: RadioIconCard currently ignores description, currentValue
and position: render the description by adding a {description &&
<FieldDescription>{description}</FieldDescription>} block (use FieldDescription
like in RadioImageCard) below the FieldTitle; apply position to the layout by
toggling the container class between "flex-row" and "flex-row-reverse" (use the
existing inner div around the icon and RadioGroupItem to conditionalize flex
direction based on position); and make currentValue meaningful by marking the
RadioGroupItem checked when props.value === currentValue (e.g., pass
checked={props.value === currentValue} to RadioGroupItem) so selection styling
is consistent with RadioImageCard. Ensure you reference RadioIconCard,
RadioGroupItem, FieldTitle and FieldDescription when making changes.

---

Outside diff comments:
In `@src/components/ui/checkbox.tsx`:
- Around line 99-107: The CheckboxCard component accepts CheckboxCardProps
(which extends LabeledCheckboxProps) and currently forwards all remaining props
via ...props so the image prop leaks onto the DOM; update the CheckboxCard
signature to explicitly destructure image (e.g., include image in the parameter
list) and do not include it in ...props so it is "sunk" and not passed into
CheckboxPrimitive.Root, or alternatively remove image from CheckboxCardProps if
card variants should not support images; ensure you update any uses of
CheckboxCard accordingly and keep CheckboxPrimitive.Root receiving only valid
DOM props.

---

Duplicate comments:
In `@src/components/settings/settings-content.tsx`:
- Around line 316-323: The issue is that isNested is passed unchanged to every
child in the element.children map, causing FieldWrapper to suppress top borders
for all children; change the prop per child so only the first child gets the
nested suppression (e.g., when rendering ElementRenderer inside the
element.children?.map, pass isNested={isNested && index === 0} instead of
isNested={isNested}) and keep the existing isGroupParent logic (isGroupParent &&
index === (element.children?.length ?? 0) - 1) unchanged so only the first child
hides its top border while others keep divide-y separators.

In `@src/components/ui/radio-group.tsx`:
- Line 162: The build fails because RawHTML (from `@wordpress/element`) requires
children of type string | string[] but the component passes label as
React.ReactNode; update the render in the RadioGroup/FieldTitle area to
type-guard label before using RawHTML (e.g., in the jsx where FieldTitle renders
RawHTML, check if typeof label === "string" or Array.isArray(label) and only
then render <RawHTML>{label}</RawHTML>; otherwise render label directly (or wrap
non-string nodes in a safe wrapper) so you don’t cast with "as any" and satisfy
RawHTML’s children type; apply the same fix at the other occurrence referenced
(line ~228).

---

Nitpick comments:
In `@src/components/ui/checkbox.tsx`:
- Around line 65-90: Extract the duplicated FieldLabel + conditional
FieldDescription JSX into a single local variable (e.g. const labelBlock) inside
the Checkbox component in src/components/ui/checkbox.tsx and use that variable
in both branches of the image conditional; ensure the extracted block still uses
props.id, label and description so the existing conditional for description
remains intact and the image branch simply renders the img plus labelBlock while
the no-image branch renders labelBlock alone.

In `@src/components/ui/radio-group.tsx`:
- Around line 127-130: Replace the use of `any` in the radio card props with
specific React-friendly types: change `currentValue: any` to a union such as
`string | number | boolean` (or the exact discriminant your radio uses) and
change `image: any` and any `icon: any` props to `React.ReactNode | string`
(string if you accept image URLs) or a more specific image prop type if you use
Next/Image props; update the `RadioImageCardProps` interface and the
corresponding prop types in the other component declarations (the other
occurrences around the icon prop) to use these types and import
`React`/`ReactNode` as needed so you preserve type safety across
`RadioImageCardProps`, `LabeledRadioProps`, and any components that consume
`currentValue`, `image`, or `icon`.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find ScrollArea implementation and usage patterns
fd -e tsx -e ts | xargs grep -l "ScrollArea" | head -10

Repository: getdokan/plugin-ui

Length of output: 221

---

🏁 Script executed:

#!/bin/bash
# Look for the ScrollArea component definition in shadcn/ui or local components
fd -e tsx -e ts -path "*/scroll-area*" | head -20

Repository: getdokan/plugin-ui

Length of output: 230

---

🏁 Script executed:

#!/bin/bash
# Search for ScrollArea root className definition
rg "overflow-hidden" -A 3 -B 3 | grep -i "scroll\|root\|classname" -A 2 -B 2 | head -50

Repository: getdokan/plugin-ui

Length of output: 3213

---

🏁 Script executed:

#!/bin/bash
# Verify the actual file in question and lines 52-53
if [ -f "src/components/settings/settings-content.tsx" ]; then
  sed -n '48,58p' src/components/settings/settings-content.tsx
fi

Repository: getdokan/plugin-ui

Length of output: 586

---

🏁 Script executed:

#!/bin/bash
# Search for ScrollArea component in the codebase (lib or ui folder)
find . -type f \( -name "*.tsx" -o -name "*.ts" \) -exec grep -l "ScrollArea.*overflow-hidden" {} \; 2>/dev/null | head -10

Repository: getdokan/plugin-ui

Length of output: 44

---

🏁 Script executed:

#!/bin/bash
# Get the full scroll-area.tsx file
cat -n src/components/ui/scroll-area.tsx

Repository: getdokan/plugin-ui

Length of output: 2205

---

🏁 Script executed:

#!/bin/bash
# Check if ScrollArea.Viewport has overflow properties
rg "Viewport" -A 5 -B 2 src/components/ui/scroll-area.tsx

Repository: getdokan/plugin-ui

Length of output: 436

---

🏁 Script executed:

#!/bin/bash
# Check how ScrollArea is used elsewhere in the codebase to understand height handling
rg "<ScrollArea" -B 2 -A 4 | head -60

Repository: getdokan/plugin-ui

Length of output: 3996

---

🏁 Script executed:

#!/bin/bash
# Check the parent container of settings-content to see if it provides height constraints
rg "settings-content\|SettingsContent" -B 5 -A 3 | head -80

Repository: getdokan/plugin-ui

Length of output: 44

---

🏁 Script executed:

#!/bin/bash
# Check if settings-content is used within a parent with height constraints
rg "<SettingsContent\|settings-content" src/ -B 10 -A 5 | head -100

Repository: getdokan/plugin-ui

Length of output: 44

---

🏁 Script executed:

#!/bin/bash
# Check the full settings-content component to understand its role and parent expectations
head -n 100 src/components/settings/settings-content.tsx

Repository: getdokan/plugin-ui

Length of output: 4493

---

🏁 Script executed:

#!/bin/bash
# Check the complete settings-content.tsx to understand the full component structure
wc -l src/components/settings/settings-content.tsx

Repository: getdokan/plugin-ui

Length of output: 110

---

🏁 Script executed:

#!/bin/bash
# Get lines around where SettingsContent closes to see the full structure
tail -n 50 src/components/settings/settings-content.tsx

Repository: getdokan/plugin-ui

Length of output: 1966

---

🏁 Script executed:

#!/bin/bash
# Search for where SettingsContent component is imported and used in the app
find src -type f \( -name "*.tsx" -o -name "*.ts" \) ! -path "*/settings-content.tsx" -exec grep -l "SettingsContent" {} \;

Repository: getdokan/plugin-ui

Length of output: 140

---

🏁 Script executed:

#!/bin/bash
# Check how Tailwind cn() utility merges conflicting overflow classes
rg "export.*cn|const cn" -A 5 src/lib/utils* 2>/dev/null || rg "export.*cn|const cn" -A 5 | head -30

Repository: getdokan/plugin-ui

Length of output: 213

---

🏁 Script executed:

#!/bin/bash
# Check how SettingsContent is wrapped/used in the parent context
cat src/components/settings/index.tsx

Repository: getdokan/plugin-ui

Length of output: 7181

---

🏁 Script executed:

#!/bin/bash
# Look at settings-context to understand how SettingsContent is mounted
head -n 150 src/components/settings/settings-context.tsx | tail -n 50

Repository: getdokan/plugin-ui

Length of output: 2238

---

🏁 Script executed:

#!/bin/bash
# Verify what twMerge does with conflicting overflow classes by checking Tailwind behavior
python3 << 'EOF'
# Simulate twMerge behavior with overflow classes
# twMerge resolves conflicts in the same Tailwind group
# overflow-hidden and overflow-y-auto are in the same overflow group
# The later one (overflow-y-auto) typically wins in CSS cascade

classes_root = "relative overflow-hidden"
classes_prop = "flex flex-col overflow-y-auto"

# twMerge merges by removing conflicting classes, keeping the later/more specific one
# In this case, overflow-y-auto (sets overflow-y: auto) is more specific than overflow-hidden
# So the result would be: relative overflow-y-auto flex flex-col

# CSS outcome:
# overflow-hidden on root from base
# overflow-y-auto on root from className prop
# Result: overflow-y-auto wins due to specificity/order
# This makes the root scrollable in Y direction, which is the issue

print("Classes from ScrollArea root base: " + classes_root)
print("Classes from className prop: " + classes_prop)
print("\nWith twMerge (last/more specific wins):")
print("Final overflow class: overflow-y-auto (overrides overflow-hidden)")
print("\nCSS Result:")
print("- Root element becomes scrollable in Y-axis")
print("- Viewport inside ALSO has scroll capability")
print("- Potential double-scroll or unexpected behavior")
EOF

Repository: getdokan/plugin-ui

Length of output: 438

---

🏁 Script executed:

#!/bin/bash
# Check if there's a parent container that should constrain height
rg "SettingsContent" src/components/settings/index.tsx -A 20 -B 5

Repository: getdokan/plugin-ui

Length of output: 1541

---

Remove overflow-y-auto from ScrollArea className—it conflicts with the base overflow-hidden root class.

The shadcn ScrollArea component sets overflow-hidden on its root element. When you pass overflow-y-auto via the className prop, twMerge resolves the conflict by keeping the more specific class (overflow-y-auto), which makes the root element itself scrollable. This bypasses the ScrollArea's internal viewport scroll mechanism, creating a double-scroll scenario (root + viewport both scroll).

ScrollArea's viewport (h-full w-full) is designed to be the sole scroller. The root should remain constrained by the parent's flex layout.

🛠️ Suggested fix

-<ScrollArea className={cn('flex flex-col overflow-y-auto', className)} data-testid="settings-content">
+<ScrollArea className={cn('flex flex-col h-full', className)} data-testid="settings-content">

📝 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

	<ScrollArea className={cn('flex flex-col overflow-y-auto', className)} data-testid="settings-content">
	<div className="flex-1">
	<ScrollArea className={cn('flex flex-col h-full', className)} data-testid="settings-content">
	<div className="flex-1">

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/settings-content.tsx` around lines 52 - 53, Remove
the conflicting overflow class from the ScrollArea call: in the JSX where
ScrollArea is rendered (the ScrollArea element using cn('flex flex-col
overflow-y-auto', className) and data-testid="settings-content"), stop including
"overflow-y-auto" so the root retains the component's default overflow-hidden;
keep the layout classes like "flex flex-col" and still merge incoming className,
and ensure the inner viewport (the element with h-full w-full) remains the only
scroller.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Icon-only doc_link anchor lacks an accessible label.

When doc_link_text is absent, the rendered anchor contains only the FileText icon with no text — screen readers cannot determine the link's purpose. Add a fallback aria-label.

🛠️ Proposed fix (applies identically at line 243)

 <a
   href={contentSource.doc_link}
   target="_blank"
   rel="noopener noreferrer"
   className="text-muted-foreground flex gap-1 items-center text-sm hover:text-foreground transition-colors shrink-0"
+  aria-label={contentSource.doc_link_text || 'Documentation'}
 >
     <FileText className="size-4" />
     { contentSource.doc_link_text ?? '' }
 </a>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/settings-content.tsx` around lines 74 - 78, The
anchor that renders the FileText icon uses contentSource.doc_link_text but falls
back to an empty string, producing an icon-only link with no accessible label;
update the anchor rendering (the <a> around FileText in settings-content.tsx) to
provide an aria-label when contentSource.doc_link_text is falsy—for example
compute a label like `${contentSource.doc_link_text ?? 'Documentation link'}` or
a localized fallback and set it on the anchor as aria-label (and keep existing
visible text when present); apply the same change to the duplicate anchor at the
other occurrence (around line 243).

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Empty sticky save bar renders when onSave is set but renderSaveButton is not provided.

showSaveArea is true whenever onSave is truthy, but lines 136–138 render nothing if renderSaveButton is absent. The result is an empty sticky footer bar at the bottom of the panel. Either gate showSaveArea on both conditions, or provide a default save button fallback.

🛠️ Proposed fix

-const showSaveArea = Boolean(onSave);
+const showSaveArea = Boolean(onSave) && Boolean(renderSaveButton);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/settings-content.tsx` around lines 131 - 140, The
sticky save bar is shown whenever showSaveArea is true (currently tied to
onSave) but renderSaveButton may be undefined, causing an empty footer; update
the condition or provide a fallback: change the visibility logic so showSaveArea
only becomes true when both onSave and renderSaveButton are present (e.g., gate
the rendering with renderSaveButton || provide a default), or implement a
default save button when renderSaveButton is undefined that calls handleSave and
uses scopeId/dirty for state; make the change around the JSX that references
showSaveArea, renderSaveButton, handleSave, scopeId and dirty so the sticky bar
is only rendered when an actual button will appear.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Nested <button> inside TooltipTrigger produces invalid HTML.

TooltipTrigger renders as a <button> by default. Placing another <button> inside it creates <button><button>…</button></button>, which is invalid HTML and will trigger React DOM-nesting warnings. Use the asChild prop to convert the trigger into a slottable area — it will replace the trigger with the child that is passed to it.

🛠️ Proposed fix

-<TooltipTrigger>
-    <button type="button" className="inline-flex">
-        <Info className="size-3.5 text-muted-foreground cursor-help" />
-    </button>
-</TooltipTrigger>
+<TooltipTrigger asChild>
+    <button type="button" className="inline-flex">
+        <Info className="size-3.5 text-muted-foreground cursor-help" />
+    </button>
+</TooltipTrigger>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/settings-content.tsx` around lines 219 - 229,
TooltipTrigger currently renders a button and you nest another <button> inside
it (the Info button), producing invalid HTML; change TooltipTrigger to use the
asChild prop so it will forward the trigger role to the inner element, i.e.,
replace TooltipTrigger with TooltipTrigger asChild and keep the existing inner
<button> (the Info icon button) as its direct child, leaving TooltipContent,
TooltipProvider and the tooltip variable usage unchanged so the trigger is the
actual button element rather than a wrapped button component.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

RawHTML on element.description is an XSS vector if the value is ever user-influenced.

@wordpress/element's RawHTML renders content via dangerouslySetInnerHTML. The type system (string) does not prevent callers from passing user-supplied or sanitized-but-unverified content. Any downstream plugin that forwards user input into description would inject arbitrary HTML/JS.

Either document that description must be developer-controlled (trusted) HTML, or sanitize before rendering:

import DOMPurify from 'dompurify';
// ...
<div
    className="text-xs text-muted-foreground leading-relaxed"
    dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(element.description) }}
/>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/settings-content.tsx` around lines 308 - 312, The use
of RawHTML to render element.description is an XSS risk if that string can ever
be user-controlled; update the rendering in the component that uses RawHTML
(reference RawHTML and element.description) to sanitize the HTML before
injecting it (e.g., use a trusted sanitizer like DOMPurify and pass the
sanitized string into dangerouslySetInnerHTML) or else document and enforce that
element.description is always developer-controlled/trusted HTML; ensure the fix
replaces the RawHTML usage with a sanitized dangerouslySetInnerHTML render or
adds validation that prevents any untrusted input from reaching
element.description.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find all occurrences of isGroupParent to understand its usage pattern
rg -n "isGroupParent" -A2 -B1 --type tsx --type ts

Repository: getdokan/plugin-ui

Length of output: 89

---

🏁 Script executed:

#!/bin/bash
# Find all occurrences of isGroupParent without specific file types
rg -n "isGroupParent" -A2 -B1

Repository: getdokan/plugin-ui

Length of output: 7651

---

isGroupParent logic is inverted at line 321.

In field-renderer.tsx (line 84), isGroupParent is set to true for all children except the last (index < length - 1). This removes bottom padding from non-final items to prevent double spacing within the group.

At line 321 in settings-content.tsx, the logic is reversed: only the last child receives isGroupParent=true (index === length - 1). This will incorrectly remove padding from the final subsection child while leaving intermediate children with default spacing, breaking visual consistency.

Change line 321 to: isGroupParent={isGroupParent && index < (element.children?.length ?? 0) - 1}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/settings-content.tsx` at line 321, The isGroupParent
prop is inverted: instead of setting it only for the last child, make it true
for every child except the last by changing the condition on isGroupParent to
check that index is less than element.children length minus one (i.e., use an
index < childrenLength - 1 comparison rather than index === childrenLength - 1);
update the prop passed where isGroupParent and index and element.children are
used so intermediate group children receive isGroupParent=true and only the
final child gets false.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check whether `@wordpress/element` is declared as a direct dependency
fd "package.json" --max-depth 2 --exclude "node_modules" | head -5
# Search for `@wordpress/element` in JSON files
rg '"@wordpress/element"' --type json -n

Repository: getdokan/plugin-ui

Length of output: 650

---

🏁 Script executed:

cat package.json | grep -A 20 '"dependencies"' | head -30
cat package.json | grep -A 20 '"peerDependencies"' | head -30
rg '@wordpress/element' package.json -A 2 -B 2

Repository: getdokan/plugin-ui

Length of output: 1392

---

Add @wordpress/element as a direct dependency.

RawHTML is imported from @wordpress/element at line 6, but @wordpress/element is not declared as a direct dependency in package.json. While it appears in package-lock.json (version ^6.39.0), it is likely a transitive dependency of @wordpress/dataviews. Relying on transitive dependencies is unreliable and may fail in consumer environments that don't have the same dependency tree. Add @wordpress/element to the dependencies section of package.json.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/ui/radio-group.tsx` at line 6, The project imports RawHTML
from `@wordpress/element` in src/components/ui/radio-group.tsx but
`@wordpress/element` is only a transitive dependency; add "@wordpress/element" to
the package.json dependencies so consumers won't break. Update package.json's
"dependencies" to include the appropriate version (e.g., ^6.39.0 or matching
your lockfile) and run npm/yarn install to update lockfiles, then verify imports
in src/components/ui/radio-group.tsx still resolve.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Duplicate group utility class.

group appears twice in the cn(...) string at line 146; the second occurrence is redundant.

🛠️ Proposed fix

-        "transition-colors has-data-checked:bg-transparent dark:has-data-checked:bg-transparent p-0 group cursor-pointer group",
+        "transition-colors has-data-checked:bg-transparent dark:has-data-checked:bg-transparent p-0 group cursor-pointer",

📝 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

	"transition-colors has-data-checked:bg-transparent dark:has-data-checked:bg-transparent p-0 group cursor-pointer group",
	"transition-colors has-data-checked:bg-transparent dark:has-data-checked:bg-transparent p-0 group cursor-pointer",

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/ui/radio-group.tsx` at line 146, The class string passed into
the cn(...) call contains the duplicate utility "group" (i.e.,
"transition-colors has-data-checked:bg-transparent
dark:has-data-checked:bg-transparent p-0 group cursor-pointer group"); remove
the second "group" occurrence so the class list only includes a single "group"
token; locate the cn(...) invocation in the radio-group component (the string
shown) and update it accordingly.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

data-testid emits "settings-field-undefined" when id is not provided.

props.id is optional (RadioPrimitive.Root.Props['id'] is string | undefined). When no id is passed, the attribute becomes settings-field-undefined, which makes test selectors unreliable.

🛠️ Proposed fix

-          data-testid={`settings-field-${props.id}`}
+          data-testid={props.id ? `settings-field-${props.id}` : undefined}

Also applies to: 209-209

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/ui/radio-group.tsx` at line 154, The data-testid attribute is
currently rendered as `settings-field-undefined` when props.id is missing;
update the RadioGroup component to only set data-testid when props.id is defined
(e.g., replace data-testid={`settings-field-${props.id}`} with a conditional
such as data-testid={props.id ? `settings-field-${props.id}` : undefined}) so
the attribute is omitted when id is absent; apply the same fix to the other
occurrence of the same expression in this file.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

RadioIconCard silently drops description; currentValue and position are accepted but never applied.

Three functional gaps relative to RadioImageCard:

1. description not rendered — RadioIconCardProps extends LabeledRadioProps, so consumers can pass description, but there is no {description && <FieldDescription>…</FieldDescription>} block anywhere in the component body. The value is silently discarded.
2. currentValue dead prop — RadioIconCard uses has-data-checked:border-primary (CSS-driven) for selection styling, so the currentValue prop is never read. If currentValue-based styling is not needed here, remove it from the interface to avoid misleading callers; otherwise apply it consistently with RadioImageCard.
3. position not applied to layout — the position prop is destructured but the inner div at line 211 has no flex-row-reverse conditional, making the prop a no-op.

🛠️ Proposed fix

 interface RadioIconCardProps extends LabeledRadioProps {
-  currentValue: any;
   icon: any;
 }

 function RadioIconCard({
   label,
   description,
   className,
   orientation = "horizontal",
   position = "left",
   disabled,
-  currentValue = '',
   icon = '',
   ...props
 }: RadioIconCardProps) {
   return (
     <FieldGroup className={cn(disabled && "opacity-50")}>
       <FieldLabel className={cn(
         "transition-colors p-4 group cursor-pointer border rounded-xl border-border",
         "has-data-checked:border-primary",
         !disabled && "hover:border-primary"
       )}>
         <Field
           orientation={orientation}
           data-disabled={disabled}
           className="flex flex-col p-0! gap-4"
           data-testid={`settings-field-${props.id}`}
         >
-          <div className="w-full flex flex-row items-center justify-between">
+          <div className={cn("w-full flex flex-row items-center justify-between", position === "right" && "flex-row-reverse")}>
             {icon && (
               <div className="size-10 flex items-center justify-center rounded-lg overflow-hidden bg-muted/30">
                 {typeof icon === 'string' ? (
                   <img src={icon} alt="" className="size-full object-contain rounded-md" />
                 ) : (
                   icon
                 )}
               </div>
             )}
             <RadioGroupItem
               className={cn("disabled:opacity-100 ml-auto", className)}
               disabled={disabled}
               {...props}
             />
           </div>
           <div className="flex gap-1 w-full">
             <FieldTitle className="font-semibold text-sm text-foreground">
               {typeof label === 'string' ? <RawHTML>{label}</RawHTML> : label}
             </FieldTitle>
           </div>
+          {description && (
+            <FieldDescription>{description}</FieldDescription>
+          )}
         </Field>
       </FieldLabel>
     </FieldGroup>
   )
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/ui/radio-group.tsx` around lines 187 - 234, RadioIconCard
currently ignores description, currentValue and position: render the description
by adding a {description && <FieldDescription>{description}</FieldDescription>}
block (use FieldDescription like in RadioImageCard) below the FieldTitle; apply
position to the layout by toggling the container class between "flex-row" and
"flex-row-reverse" (use the existing inner div around the icon and
RadioGroupItem to conditionalize flex direction based on position); and make
currentValue meaningful by marking the RadioGroupItem checked when props.value
=== currentValue (e.g., pass checked={props.value === currentValue} to
RadioGroupItem) so selection styling is consistent with RadioImageCard. Ensure
you reference RadioIconCard, RadioGroupItem, FieldTitle and FieldDescription
when making changes.