Conversation
… functionality and adjust styles. Update event handling for component deletion via keyboard shortcuts. Clean up unused styles related to component controls.
…or improved usability.
…changes, improving user experience. Remove previous autoFocus effect on tab change.
…inside the settings panel. Refactor SearchableTabs component to streamline search input handling and improve styling with added margin.
WalkthroughCentralized deletion from the drag wrapper to the designer main area with a window-level Delete/Backspace key handler and contextual focus checks; removed wrapper delete UI and related CSS. Refactored searchable tabs to use a single shared search input ref and simplified tab rendering and form state usage. Changes
Sequence DiagramsequenceDiagram
participant User
participant Designer as Designer Main Area
participant DeleteHandler as deleteComponent
participant Store as Form State
User->>Designer: Press Delete / Backspace
Designer->>Designer: Ensure mode=designer && !readOnly
Designer->>Designer: Ignore if event from input/textarea/contenteditable
Designer->>Designer: Ignore if settingsPanelRef contains event target
Designer->>Designer: Check selectedComponentId exists
Designer->>DeleteHandler: deleteComponent({ componentId })
DeleteHandler->>Store: Remove component from state
DeleteHandler->>Designer: Clear selection / emit update
Designer->>User: UI updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shesha-reactjs/src/designer-components/propertiesTabs/searchableTabsComponent.tsx (1)
88-140:⚠️ Potential issue | 🟠 Major
newFilteredTabsis recomputed (new array reference) on every render, so bothuseEffects below run every render.Previously a
useMemo/localTabswrapped this computation; it was removed in this PR. BecausenewFilteredTabsis now a fresh array instance on each render and is listed in the dependency arrays of the effects on lines 133 and 140, those effects fire on every render. In addition to the wasted work (re-running.find/.somescans, allocating new arrays, and iteratingtabs+filterDynamicComponents), any code path that would callsetActiveTabKeyinside these effects risks a render storm if the condition briefly becomes unstable. Consider restoring memoization:♻️ Proposed refactor
-import React, { useEffect, useState } from 'react'; +import React, { useEffect, useMemo, useState } from 'react'; @@ - const newFilteredTabs = tabs - .map((tab: any, index: number) => { + const newFilteredTabs = useMemo(() => tabs + .map((tab, index: number) => { ... }) - .filter((tab) => !tab.hidden); + .filter((tab) => !tab.hidden), + [tabs, searchQuery, model, formState?.name] + );Also note: inside
isComponentHidden(line 77) the component'sinputsarray is mutated in place (component.inputs = visibleInputs), so re-running on every render also progressively mutates the source model. That mutation should be avoided regardless — clone before filtering.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shesha-reactjs/src/designer-components/propertiesTabs/searchableTabsComponent.tsx` around lines 88 - 140, newFilteredTabs is being recomputed every render which causes both useEffect blocks that depend on it to run every render; wrap the tabs -> newFilteredTabs computation in useMemo (dependent on tabs, searchQuery, model as needed) so the array reference is stable between renders and avoid unnecessary effects and potential render storms when calling setActiveTabKey; additionally fix isComponentHidden to avoid mutating component.inputs in place by cloning the component or its inputs before filtering (e.g., create a local visibleInputs array and use that for checks without assigning back to component.inputs) and ensure filterDynamicComponents is used inside the memoized computation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@shesha-reactjs/src/designer-components/propertiesTabs/searchableTabsComponent.tsx`:
- Line 89: The map callback uses `tab: any` (and there are other implicit anys:
the ref callback `el: any` and the `component` parameter), so replace those anys
with a concrete tab type by importing/using the existing shape: use
IPropertiesTabsComponentProps['tabs'][number] (or define a local TabItem
interface) for the `tab` parameter in .map, annotate the ref callback with the
appropriate DOM/ref type instead of `any`, and type the `component` parameter
similarly (or use unknown and narrow as needed); update the function signatures
in searchableTabsComponent.tsx (e.g., the map callback, the ref callback, and
the component handler) to use these types so TypeScript infers correctly and
eliminates implicit anys.
- Around line 61-63: The handleTabChange currently only calls setActiveTabKey
and does not imperatively focus the search input as the PR promises; fix by
creating a ref for the search Input (e.g., searchInputRef) used where the
<Input> is rendered, remove relying on changing autoFocus after mount (autoFocus
only runs on mount), and update handleTabChange to call
searchInputRef.current?.focus() (or focus input wrapper) immediately after
setActiveTabKey; also ensure the <Input> uses the ref and keep or remove the
existing autoFocus: newFilteredTabs.length === 0 logic as appropriate for
initial mount behavior.
---
Outside diff comments:
In
`@shesha-reactjs/src/designer-components/propertiesTabs/searchableTabsComponent.tsx`:
- Around line 88-140: newFilteredTabs is being recomputed every render which
causes both useEffect blocks that depend on it to run every render; wrap the
tabs -> newFilteredTabs computation in useMemo (dependent on tabs, searchQuery,
model as needed) so the array reference is stable between renders and avoid
unnecessary effects and potential render storms when calling setActiveTabKey;
additionally fix isComponentHidden to avoid mutating component.inputs in place
by cloning the component or its inputs before filtering (e.g., create a local
visibleInputs array and use that for checks without assigning back to
component.inputs) and ensure filterDynamicComponents is used inside the memoized
computation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4cd4ea6f-d5d6-467a-b513-dc0a34c94618
📒 Files selected for processing (5)
shesha-reactjs/src/components/formDesigner/configurableFormComponent/dragWrapper.tsxshesha-reactjs/src/components/formDesigner/designerMainArea/index.tsxshesha-reactjs/src/components/formDesigner/styles/styles.tsshesha-reactjs/src/designer-components/propertiesTabs/searchableTabsComponent.tsxshesha-reactjs/src/designer-components/propertiesTabs/style.ts
…focus management, enhancing user experience. Update type definitions for better type safety and improve handling of dynamic components in tab rendering.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
shesha-reactjs/src/designer-components/propertiesTabs/searchableTabsComponent.tsx (2)
94-146:⚠️ Potential issue | 🟠 Major
newFilteredTabsis recomputed every render and drives twouseEffectdependency arrays.Per the PR summary the previous
useMemowrapper was removed, sonewFilteredTabsis now a brand-new array reference on every render. That reference change re-fires both effects at lines 130-139 and 142-146 on every render (only the conditionalsetActiveTabKeyguards against a loop) and also rebuilds the JSX children (<ParentProvider>…</ParentProvider>) anditemsarray passed to<Tabs>every pass. Wrap the computation inuseMemoso effect dependencies and<Tabs items>remain stable unlesstabs/searchQuery/formState/formActions/modelactually change.♻️ Suggested change
- const newFilteredTabs = tabs - .map((tab: TabItem, index: number) => { + const newFilteredTabs = useMemo(() => tabs + .map((tab: TabItem, index: number) => { ... - }) - .filter((tab) => !tab.hidden); + }) + .filter((tab) => !tab.hidden), + [tabs, searchQuery, model, formState, formActions, styles]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shesha-reactjs/src/designer-components/propertiesTabs/searchableTabsComponent.tsx` around lines 94 - 146, The newFilteredTabs array is being rebuilt on every render which invalidates effects and children; wrap the computation that builds newFilteredTabs (the map/filter logic that calls filterDynamicComponents, isComponentHidden and constructs ComponentsContainer/ParentProvider JSX) in a useMemo and list its real dependencies: tabs, searchQuery, formState, formActions, and model so the array and the Tabs items remain referentially stable; keep existing variable names (newFilteredTabs, tabs, searchQuery, model, filterDynamicComponents, isComponentHidden, ComponentsContainer, ParentProvider) and ensure useEffect deps (searchQuery, newFilteredTabs, activeTabKey) now receive the memoized newFilteredTabs.
69-90:⚠️ Potential issue | 🟠 MajorAvoid mutating the incoming
component.inputsduring render.
isComponentHiddenis invoked from inside.mapduring render (line 99) and at line 81 it reassignscomponent.inputs = visibleInputson the live object that comes from thetabsprop. That violates render purity and permanently shrinks the stored inputs array for subsequent renders (the next pass only sees the already-filtered subset). Compute visibility without mutating the input; either return the filtered inputs alongside the boolean or make a shallow copy before filtering.🛠 Suggested change
- const isComponentHidden = (component: IConfigurableFormComponent & { inputs?: IConfigurableFormComponent[] }): boolean => { - if (formState.name === "modalSettings") { - if (component.inputs) { - const visibleInputs = component.inputs.filter((input) => { - if (!input.propertyName) return true; - return formActions.isComponentFiltered(input); - }); - - if (visibleInputs.length === 0) { - return false; - } - - component.inputs = visibleInputs; - - return visibleInputs.length > 0; - } - - return formActions.isComponentFiltered(component); - } else { - return true; - } - }; + const getVisibleComponent = ( + component: IConfigurableFormComponent & { inputs?: IConfigurableFormComponent[] }, + ): (IConfigurableFormComponent & { inputs?: IConfigurableFormComponent[] }) | null => { + if (formState.name !== "modalSettings") return component; + if (component.inputs) { + const visibleInputs = component.inputs.filter( + (input) => !input.propertyName || formActions.isComponentFiltered(input), + ); + return visibleInputs.length === 0 ? null : { ...component, inputs: visibleInputs }; + } + return formActions.isComponentFiltered(component) ? component : null; + };and update the caller at line 98-100 to map-then-filter with this helper.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shesha-reactjs/src/designer-components/propertiesTabs/searchableTabsComponent.tsx` around lines 69 - 90, isComponentHidden currently mutates the incoming component by assigning component.inputs = visibleInputs; instead compute visibility without mutation: inside isComponentHidden (and where you filter inputs) create a local const visibleInputs = (component.inputs || []).filter(...) and do not assign back to component.inputs, return the boolean based on visibleInputs.length and/or return both isHidden and the filtered inputs (or change the caller that maps tabs to first map each component to {component, filteredInputs} then filter by filteredInputs.length) so the tabs prop is never mutated; keep references to isComponentHidden, component.inputs, and formActions.isComponentFiltered to locate the logic and update the map/filter callsite accordingly.
♻️ Duplicate comments (1)
shesha-reactjs/src/designer-components/propertiesTabs/searchableTabsComponent.tsx (1)
151-155:⚠️ Potential issue | 🟡 Minor
autoFocustoggle is a no-op after mount.The same
<Input>element persists across renders (it is not re-keyed or unmounted), soautoFocusonly fires on initial mount — flipping it based onnewFilteredTabs.length === 0later has no effect. The ref-basedrequestAnimationFramefocus inhandleTabChangeis now the real mechanism. Either drop theautoFocusprop to avoid the misleading appearance of dynamic focus, or key the Input on the empty/non-empty state if you actually want it to remount.🧹 Suggested cleanup
{renderSearchInput({ ref: searchInputRef, - autoFocus: newFilteredTabs.length === 0, className: styles.searchField, })}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shesha-reactjs/src/designer-components/propertiesTabs/searchableTabsComponent.tsx` around lines 151 - 155, The autoFocus prop passed into renderSearchInput is a no-op after mount because the same Input instance persists; update the implementation to either remove the autoFocus prop (so focus is managed exclusively via the ref + requestAnimationFrame logic in handleTabChange and searchInputRef) or make the Input remount by adding a key that depends on newFilteredTabs.length === 0 (e.g., key={`empty-${newFilteredTabs.length===0}`}), and keep focus handling consistent in renderSearchInput/handleTabChange accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@shesha-reactjs/src/designer-components/propertiesTabs/searchableTabsComponent.tsx`:
- Around line 94-146: The newFilteredTabs array is being rebuilt on every render
which invalidates effects and children; wrap the computation that builds
newFilteredTabs (the map/filter logic that calls filterDynamicComponents,
isComponentHidden and constructs ComponentsContainer/ParentProvider JSX) in a
useMemo and list its real dependencies: tabs, searchQuery, formState,
formActions, and model so the array and the Tabs items remain referentially
stable; keep existing variable names (newFilteredTabs, tabs, searchQuery, model,
filterDynamicComponents, isComponentHidden, ComponentsContainer, ParentProvider)
and ensure useEffect deps (searchQuery, newFilteredTabs, activeTabKey) now
receive the memoized newFilteredTabs.
- Around line 69-90: isComponentHidden currently mutates the incoming component
by assigning component.inputs = visibleInputs; instead compute visibility
without mutation: inside isComponentHidden (and where you filter inputs) create
a local const visibleInputs = (component.inputs || []).filter(...) and do not
assign back to component.inputs, return the boolean based on
visibleInputs.length and/or return both isHidden and the filtered inputs (or
change the caller that maps tabs to first map each component to {component,
filteredInputs} then filter by filteredInputs.length) so the tabs prop is never
mutated; keep references to isComponentHidden, component.inputs, and
formActions.isComponentFiltered to locate the logic and update the map/filter
callsite accordingly.
---
Duplicate comments:
In
`@shesha-reactjs/src/designer-components/propertiesTabs/searchableTabsComponent.tsx`:
- Around line 151-155: The autoFocus prop passed into renderSearchInput is a
no-op after mount because the same Input instance persists; update the
implementation to either remove the autoFocus prop (so focus is managed
exclusively via the ref + requestAnimationFrame logic in handleTabChange and
searchInputRef) or make the Input remount by adding a key that depends on
newFilteredTabs.length === 0 (e.g.,
key={`empty-${newFilteredTabs.length===0}`}), and keep focus handling consistent
in renderSearchInput/handleTabChange accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 46b00199-7636-44de-8efd-9e56ea1ebff9
📒 Files selected for processing (1)
shesha-reactjs/src/designer-components/propertiesTabs/searchableTabsComponent.tsx
1 participant
#4748
Summary by CodeRabbit
New Features
Improvements