Conversation
…into derik/task/3459
…into derik/task/3459
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. WalkthroughAdds per-item styling hooks (itemStyle, menuItemStyle, containerStyle, menuItemShadow), extends menu color model with sub-item fields, threads these through style generators, designer settings and migration, updates overflow typing/handling, and refactors menu selection to traverse hierarchical sidebar items. Changes
Sequence Diagram(s)(Skipped — changes extend styling/model surface and selection recursion without introducing a new multi-component sequential runtime flow needing diagramming.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 4
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/components/menu/index.tsx (1)
34-45: Forward new style props into the drawer flow.
itemStyle/menuItemStyleare added to the public API but are not passed toShaMenuDrawerwhenoverflow === "menu", so the new styling has no effect in that mode.🛠️ Suggested fix (propagate to drawer)
<ShaMenuDrawer items={menuItems} open={open} onClose={onClose} colors={colors} fontStyles={fontStyles} styleOnHover={styleOnHover} styleOnSelected={styleOnSelected} styleOnSubMenu={styleOnSubMenu} + itemStyle={itemStyle} + menuItemStyle={menuItemStyle} menuId={menuId} />
🤖 Fix all issues with AI agents
In `@shesha-reactjs/src/designer-components/_common-migrations/migrateStyles.ts`:
- Around line 95-103: The migration currently overwrites existing menuItemShadow
and treats overflow with || which drops deliberate false; update the
menuItemShadow construction in migrateStyles (menuItemShadow) to use nullish
coalescing (??) and fall back to prev?.menuItemShadow's individual properties
when defaults properties are null/undefined (e.g., offsetX:
defaults?.menuItemShadow?.offsetX ?? prev?.menuItemShadow?.offsetX ?? 0), and
change overflow to use nullish coalescing (overflow: defaults?.overflow ??
prev?.overflow) so false is preserved; keep the display spread logic as-is.
In `@shesha-reactjs/src/designer-components/horizontalMenu/index.tsx`:
- Around line 100-103: The normalization misses decimal and whitespace-only
numbers; update the rawWidth/width logic to first coerce rawWidth to a trimmed
string, then treat either numeric types or strings that match /^\d+(\.\d+)?$/
(allowing decimals) as unitless and append "px"; otherwise keep the original
string. Reference rawWidth and width in index.tsx and ensure you handle number
values (e.g., typeof rawWidth === "number" or Number.isFinite(parseFloat(...)))
and trim() before testing.
In `@shesha-reactjs/src/designer-components/horizontalMenu/settings.ts`:
- Around line 395-430: Replace the unsafe "as any" casts on the readOnly
expressions inside the addSettingsInput calls for propertyName 'style',
'styleOnHover', 'styleOnSelected', and 'styleOnSubMenu' by creating a properly
typed IPropertySetting<boolean> value and passing it into the readOnly field;
locate the four addSettingsInput(...) calls and instead of readOnly: { _code:
'return getSettingValue(data?.readOnly);', _mode: 'code', _value: false } as
any, define a typed constant (e.g. const readOnlyExpr: IPropertySetting<boolean>
= { _code: 'return getSettingValue(data?.readOnly);', _mode: 'code', _value:
false }) and use readOnly: readOnlyExpr so the expressions are strongly typed
without any casts.
In `@shesha-reactjs/src/utils/index.ts`:
- Around line 19-24: Replace the unsafe cast in findSelectedItem by using the
existing type guard: import isSidebarGroup from "@/interfaces/sidebar" (or use
the existing export) and use isSidebarGroup(item) to check for a group before
accessing childItems; update the block where you currently do const group = item
as ISidebarGroup and subsequent access to group.childItems to instead branch on
isSidebarGroup(item) and then use item.childItems, preserving the recursive call
and parentIds behavior.
shesha-reactjs/src/designer-components/_common-migrations/migrateStyles.ts
Show resolved
Hide resolved
| .addSettingsInput({ | ||
| id: nanoid(), | ||
| inputType: 'codeEditor', | ||
| propertyName: 'style', | ||
| label: 'Style', | ||
| mode: 'dialog', | ||
| description: 'A script that returns the style of the element as an object. This should conform to CSSProperties', | ||
| readOnly: { _code: 'return getSettingValue(data?.readOnly);', _mode: 'code', _value: false } as any, | ||
| }) | ||
| .addSettingsInput({ | ||
| id: nanoid(), | ||
| inputType: 'codeEditor', | ||
| propertyName: 'styleOnHover', | ||
| label: 'Style On Hover', | ||
| mode: 'dialog', | ||
| description: 'A script that returns the hover style of the element as an object. This should conform to CSSProperties', | ||
| readOnly: { _code: 'return getSettingValue(data?.readOnly);', _mode: 'code', _value: false } as any, | ||
| }) | ||
| .addSettingsInput({ | ||
| id: nanoid(), | ||
| inputType: 'codeEditor', | ||
| propertyName: 'styleOnSelected', | ||
| label: 'Style On Selected', | ||
| mode: 'dialog', | ||
| description: 'A script that returns the selected style of the element as an object. This should conform to CSSProperties', | ||
| readOnly: { _code: 'return getSettingValue(data?.readOnly);', _mode: 'code', _value: false } as any, | ||
| }) | ||
| .addSettingsInput({ | ||
| id: nanoid(), | ||
| inputType: 'codeEditor', | ||
| propertyName: 'styleOnSubMenu', | ||
| label: 'Style On Sub Menu', | ||
| mode: 'dialog', | ||
| description: 'A script that returns the sub menu style of the element as an object. This should conform to CSSProperties', | ||
| readOnly: { _code: 'return getSettingValue(data?.readOnly);', _mode: 'code', _value: false } as any, | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's find where readOnly property is typed
rg -A 5 "readOnly\s*\?:" shesha-reactjs/src --max-count=20Repository: shesha-io/shesha-framework
Length of output: 196
🏁 Script executed:
#!/bin/bash
# Look for Expression types and code expression patterns
rg "type.*Expression|interface.*Expression" shesha-reactjs/src -B 2 -A 5 --max-count=20Repository: shesha-io/shesha-framework
Length of output: 12701
🏁 Script executed:
#!/bin/bash
# Check if there's a CodeExpression or similar type already defined
rg "_mode|_code|_value" shesha-reactjs/src --max-count=30 | head -40Repository: shesha-io/shesha-framework
Length of output: 5690
🏁 Script executed:
#!/bin/bash
# Examine the actual file in question
cat -n shesha-reactjs/src/designer-components/horizontalMenu/settings.ts | sed -n '390,435p'Repository: shesha-io/shesha-framework
Length of output: 3410
🏁 Script executed:
#!/bin/bash
# Find the PropertySettingMode type and the context around _code/_mode/_value
rg -B 5 -A 10 "PropertySettingMode|_mode.*PropertySettingMode" shesha-reactjs/src --max-count=20Repository: shesha-io/shesha-framework
Length of output: 6465
🏁 Script executed:
#!/bin/bash
# Look for type definitions that include _code, _mode, _value structure
rg -B 3 -A 8 "interface.*{.*_code|type.*{.*_code" shesha-reactjs/src --max-count=20Repository: shesha-io/shesha-framework
Length of output: 198
🏁 Script executed:
#!/bin/bash
# Check the settings builder interface to see what readOnly property expects
rg -B 5 -A 5 "addSettingsInput|ISettingsConfiguration" shesha-reactjs/src --max-count=20Repository: shesha-io/shesha-framework
Length of output: 50384
🏁 Script executed:
#!/bin/bash
# Look at the form models file to understand the type structure better
cat -n shesha-reactjs/src/providers/form/models.ts | head -100Repository: shesha-io/shesha-framework
Length of output: 4267
🏁 Script executed:
#!/bin/bash
# Find the SettingsInputComponentProps type to see what readOnly expects
rg -B 3 -A 15 "interface SettingsInputComponentProps|type SettingsInputComponentProps" shesha-reactjs/srcRepository: shesha-io/shesha-framework
Length of output: 908
🏁 Script executed:
#!/bin/bash
# Look at other similar settings files to see if they have better typing approaches
rg -B 2 -A 8 "readOnly.*_mode.*_code" shesha-reactjs/src/designer-components --max-count=10Repository: shesha-io/shesha-framework
Length of output: 1459
🏁 Script executed:
#!/bin/bash
# Find ISettingsInputProps interface to see readOnly type
rg -B 5 -A 20 "interface ISettingsInputProps" shesha-reactjs/src/designer-components/settingsInput/interfaces.tsRepository: shesha-io/shesha-framework
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Check if there are type definitions for readOnly in the settings input component
rg -B 3 -A 3 "readOnly\s*\?.*IPropertySetting|readOnly\s*\?.*PropertySetting" shesha-reactjs/srcRepository: shesha-io/shesha-framework
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Find the ISettingsInputProps definition with broader search
rg -B 2 -A 25 "ISettingsInputProps" shesha-reactjs/src --max-count=5Repository: shesha-io/shesha-framework
Length of output: 14645
🏁 Script executed:
#!/bin/bash
# Look at the settingsInput interfaces file more broadly
cat -n shesha-reactjs/src/designer-components/settingsInput/interfaces.ts | head -80Repository: shesha-io/shesha-framework
Length of output: 3813
🏁 Script executed:
#!/bin/bash
# Check if readOnly property type in BaseInputProps or IConfigurableFormComponent supports IPropertySetting
rg -B 5 -A 5 "readOnly\s*\?:\s*boolean\s*\|.*IPropertySetting|readOnly\s*\?:.*IPropertySetting" shesha-reactjs/srcRepository: shesha-io/shesha-framework
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Look at IConfigurableFormComponent to see the readOnly type definition
rg -B 3 -A 20 "interface IConfigurableFormComponent" shesha-reactjs/src --max-count=3Repository: shesha-io/shesha-framework
Length of output: 11614
🏁 Script executed:
#!/bin/bash
# Check how readOnly is actually evaluated/processed in the settingsInput component
rg -B 3 -A 8 "isPropertySettings.*readOnly|readOnly.*isPropertySettings|readOnly.*_mode" shesha-reactjs/src/designer-components --max-count=15Repository: shesha-io/shesha-framework
Length of output: 50382
Remove as any casts on readOnly/hidden expressions and use explicit IPropertySetting typing.
The code uses multiple as any casts for code expression objects that match the IPropertySetting<boolean> structure. Per project guidelines, eliminate any types and leverage TypeScript's type system. Define these expressions with proper types instead of unsafe casts:
♻️ Example refactor (apply to all four instances)
+const readOnlyExpr: IPropertySetting<boolean> = {
+ _code: 'return getSettingValue(data?.readOnly);',
+ _mode: 'code',
+ _value: false,
+};
...
- readOnly: { _code: 'return getSettingValue(data?.readOnly);', _mode: 'code', _value: false } as any,
+ readOnly: readOnlyExpr,🤖 Prompt for AI Agents
In `@shesha-reactjs/src/designer-components/horizontalMenu/settings.ts` around
lines 395 - 430, Replace the unsafe "as any" casts on the readOnly expressions
inside the addSettingsInput calls for propertyName 'style', 'styleOnHover',
'styleOnSelected', and 'styleOnSubMenu' by creating a properly typed
IPropertySetting<boolean> value and passing it into the readOnly field; locate
the four addSettingsInput(...) calls and instead of readOnly: { _code: 'return
getSettingValue(data?.readOnly);', _mode: 'code', _value: false } as any, define
a typed constant (e.g. const readOnlyExpr: IPropertySetting<boolean> = { _code:
'return getSettingValue(data?.readOnly);', _mode: 'code', _value: false }) and
use readOnly: readOnlyExpr so the expressions are strongly typed without any
casts.
There was a problem hiding this comment.
Actionable comments posted: 1
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/horizontalMenu/index.tsx (1)
132-140: Avoidas anytype cast; use proper CSS type.Per coding guidelines, eliminate
anytypes. UseCSSProperties['textAlign']for thetextAlignproperty.♻️ Proposed fix
const finalFontStyles = useMemo(() => { return { fontSize: model?.font?.size ? `${model.font.size}px` : `${fontSize}px`, fontFamily: model?.font?.type, fontWeight: model?.font?.weight as CSSProperties['fontWeight'], color: model?.font?.color, - textAlign: model?.font?.align as any, + textAlign: model?.font?.align as CSSProperties['textAlign'], }; }, [model.font, fontSize]);
🤖 Fix all issues with AI agents
In `@shesha-reactjs/src/utils/index.ts`:
- Around line 36-45: The null-check bug is that `typeof formId === 'object'`
will be true for null; update the guard around
item.actionConfiguration?.actionArguments?.formId in the navType === 'form'
branch so you only call getDynamicPath when formId is non-null and an object
(e.g., check formId !== null && typeof formId === 'object' or use a truthy
object guard). Specifically modify the logic that reads const formId =
item.actionConfiguration?.actionArguments?.formId and the conditional that calls
getDynamicPath(formId) to include the explicit null check before invoking
getDynamicPath.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@shesha-reactjs/src/utils/index.ts`:
- Around line 14-17: The current expression path?.split('?')[0]?.split('#')[0]
can throw because optional chaining doesn't protect the subsequent [0] access;
update the logic around pathWithoutQuery/normalizedPath so you only call split
when path is non-nullish (e.g., check path != null before splitting) and pass a
safe string or undefined into normalizeUrl; specifically, modify the computation
of pathWithoutQuery (the variable referenced) to short-circuit when path is
null/undefined and then use normalizeUrl(normalizedInput) so normalizeUrl is
never invoked with an undefined result.
- Around line 36-41: The current check only ensures formId is an object but not
that it conforms to IReferenceListIdentifier; add a guard before calling
getDynamicPath to verify item.actionConfiguration?.actionArguments?.formId has
non-empty string properties module and name (e.g., typeof formId.module ===
'string' && formId.module && typeof formId.name === 'string' && formId.name) or
extract this into an isReferenceListIdentifier(type guard) and use it in the
navType === 'form' branch; if the guard fails, avoid calling getDynamicPath and
handle the fallback (skip building itemUrl or set a safe default) so
getDynamicPath is never invoked with an invalid shape.
- Around line 48-50: The code that strips query/hash from itemUrl uses
itemUrl?.split('?')[0]?.split('#')[0] which can throw if split returns
undefined; update the chain to use optional chaining on the array access so you
read the first element safely (e.g., use itemUrl?.split('?')?.[0] and then
split('#')?.[0]) before passing to normalizeUrl; modify the logic that assigns
itemUrlWithoutQuery and normalizedItemUrl to use these ?.[] accesses to avoid a
TypeError while still calling normalizeUrl(itemUrlWithoutQuery).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@shesha-reactjs/src/utils/index.ts`:
- Around line 10-15: The current type guard isReferenceListIdentifier checks
existence of 'module' and 'name' but not their types; update it to also verify
that value.module and value.name are strings (e.g., after confirming value is a
non-null object and not an array, add typeof value.module === 'string' && typeof
value.name === 'string') so only valid IReferenceListIdentifier shapes pass;
keep the same function name and predicate return type.
shesha-reactjs/src/utils/index.ts
Outdated
| const isReferenceListIdentifier = (value: unknown): value is IReferenceListIdentifier => | ||
| typeof value === 'object' && | ||
| value !== null && | ||
| !Array.isArray(value) && | ||
| 'module' in value && | ||
| 'name' in value; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider strengthening the type guard to verify property types.
The guard confirms module and name exist but doesn't verify they are strings. If IReferenceListIdentifier expects string properties, invalid shapes like { module: 123, name: null } would pass.
♻️ Stricter type guard
const isReferenceListIdentifier = (value: unknown): value is IReferenceListIdentifier =>
typeof value === 'object' &&
value !== null &&
!Array.isArray(value) &&
- 'module' in value &&
- 'name' in value;
+ 'module' in value &&
+ typeof (value as Record<string, unknown>).module === 'string' &&
+ 'name' in value &&
+ typeof (value as Record<string, unknown>).name === '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.
| const isReferenceListIdentifier = (value: unknown): value is IReferenceListIdentifier => | |
| typeof value === 'object' && | |
| value !== null && | |
| !Array.isArray(value) && | |
| 'module' in value && | |
| 'name' in value; | |
| const isReferenceListIdentifier = (value: unknown): value is IReferenceListIdentifier => | |
| typeof value === 'object' && | |
| value !== null && | |
| !Array.isArray(value) && | |
| 'module' in value && | |
| typeof (value as Record<string, unknown>).module === 'string' && | |
| 'name' in value && | |
| typeof (value as Record<string, unknown>).name === 'string'; |
🤖 Prompt for AI Agents
In `@shesha-reactjs/src/utils/index.ts` around lines 10 - 15, The current type
guard isReferenceListIdentifier checks existence of 'module' and 'name' but not
their types; update it to also verify that value.module and value.name are
strings (e.g., after confirming value is a non-null object and not an array, add
typeof value.module === 'string' && typeof value.name === 'string') so only
valid IReferenceListIdentifier shapes pass; keep the same function name and
predicate return type.
2 participants
#3459
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.