Skip to content

Refactor styles and improve layout logic#4428

Merged
James-Baloyi merged 3 commits intoshesha-io:mainfrom
czwe-01:thulasizwe/b/89997
Jan 21, 2026
Merged

Refactor styles and improve layout logic#4428
James-Baloyi merged 3 commits intoshesha-io:mainfrom
czwe-01:thulasizwe/b/89997

Conversation

@czwe-01
Copy link
Copy Markdown
Collaborator

@czwe-01 czwe-01 commented Jan 15, 2026

  • Removed z-index override for ant-tabs-dropdown in GlobalSheshaStyles.
  • Added justify-content property to toolbar button styles in propertiesTabs.
  • Updated auto zoom calculation logic to determine main area index based on sizes array length.
  • Set zIndexPopup for Tabs component in ThemeProvider.

Summary by CodeRabbit

  • Style

    • Removed an explicit z-index override for dropdowns to simplify stacking.
    • Adjusted flex alignment in the properties toolbar for more consistent button layout.
    • Added theme configuration to raise Tabs popup stacking priority.
  • Refactor

    • Canvas width calculation now adapts to panel count for more accurate auto-zoom behavior.

✏️ Tip: You can customize this high-level summary in your review settings.

- Removed z-index override for ant-tabs-dropdown in GlobalSheshaStyles.
- Added justify-content property to toolbar button styles in propertiesTabs.
- Updated auto zoom calculation logic to determine main area index based on sizes array length.
- Set zIndexPopup for Tabs component in ThemeProvider.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Jan 15, 2026

Walkthrough

Removed an explicit .ant-tabs-dropdown z-index rule, added Tabs.zIndexPopup in the theme, adjusted toolbar button flex alignment, and made canvas available-width selection use a dynamic mainAreaIndex based on sizes length.

Changes

Cohort / File(s) Summary
Ant Tabs stacking & theme
shesha-reactjs/src/components/mainLayout/styles/indexStyles.ts, shesha-reactjs/src/providers/theme/index.tsx
Deleted the z-index: 1250 rule for body > div .ant-tabs-dropdown; added components.Tabs.zIndexPopup in the Ant Design theme configuration.
Toolbar button flex alignment
shesha-reactjs/src/designer-components/propertiesTabs/style.ts
Added justify-content: normal; to .sha-toolbar-btn-configurable and .ant-btn selectors.
Canvas width / auto-zoom logic
shesha-reactjs/src/providers/canvas/utils.ts
Replaced sizes[1] with sizes[mainAreaIndex] where mainAreaIndex = 0 if sizes.length <= 2, else 1; changes available width used for auto-zoom calculations.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • IvanIlyichev

"I'm a rabbit in the code, nibbling at z-index hay,
Buttons line up neatly as I hop along the way,
Canvas finds its center where the sizes play,
Popups stack politely, no more overlap fray,
I hop, I patch, I tidy—hip-hip-hop hooray!" 🐇✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Refactor styles and improve layout logic' accurately reflects the main changes: style refactoring across multiple files and layout logic improvements in the canvas utils.

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

✨ Finishing touches
  • 📝 Generate docstrings


📜 Recent review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 77e59dd and 551a496.

📒 Files selected for processing (1)
  • shesha-reactjs/src/providers/theme/index.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CodeRabbitReview-Project-Specific-Guidelines.md)

**/*.{ts,tsx}: Eliminate the any type; use unknown type instead for values with unknown types, forcing explicit type checking
Prefer type guards over type casting for type checking
Avoid monolithic types; use discriminated unions with a discriminator property instead
Leverage TypeScript to its full potential as a type system, not merely as a linter

Files:

  • shesha-reactjs/src/providers/theme/index.tsx
🧠 Learnings (14)
📓 Common learnings
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4084
File: shesha-reactjs/src/components/quickView/styles/styles.ts:41-94
Timestamp: 2025-10-28T14:21:55.782Z
Learning: In shesha-reactjs/src/components/quickView/styles/styles.ts, the quickViewContent styles intentionally use multiple `!important` flags and broad selectors (including spans, form items, inputs, links, and labels) to ensure proper targeting and prevent layout breakage in the quick view component.
📚 Learning: 2025-10-02T11:25:33.370Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3926
File: shesha-reactjs/src/designer-components/dataTable/tableContext/styles.ts:138-166
Timestamp: 2025-10-02T11:25:33.370Z
Learning: In the shesha-framework repository, the hint popover components (sha-quick-search-hint-popover, sha-table-view-selector-hint-popover, sha-table-pager-hint-popover) in shesha-reactjs/src/designer-components/dataTable/tableContext/styles.ts intentionally use hard-coded color `rgb(214, 214, 214)` with `!important` to impose a specific consistent color across themes, overriding theme tokens by design.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-11-05T08:12:08.149Z
Learnt from: Lihlu
Repo: shesha-io/shesha-framework PR: 4132
File: shesha-reactjs/src/providers/layersProvider/index.tsx:35-39
Timestamp: 2025-11-05T08:12:08.149Z
Learning: In shesha-reactjs LayerGroupConfiguratorProvider (src/providers/layersProvider/index.tsx), the provider uses a unidirectional data flow pattern after initialization: props.items provide initial state to the reducer (line 35-38), user actions modify the reducer state internally, and changes sync back to the parent via onChange callbacks in consuming components (e.g., modal.tsx line 55-57 uses useDeepCompareEffect). Do NOT add a useEffect to sync props.items back to the reducer state, as this creates a feedback loop that breaks layer updates in the Calendar UI. The pattern is intentionally "controlled initialization, uncontrolled updates" where the provider manages state internally after the initial mount.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-12-02T14:32:10.827Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4280
File: shesha-reactjs/src/designer-components/dataTable/tableContext/styles.ts:205-227
Timestamp: 2025-12-02T14:32:10.827Z
Learning: In shesha-reactjs/src/designer-components/dataTable/tableContext/styles.ts, the mockup components (filterButtonMockup and viewSelectorMockup) intentionally use hardcoded colors (`#d9d9d9`, `#fafafa`, `#8c8c8c`) to maintain consistent appearance across themes, overriding theme tokens by design.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-06-10T11:52:51.462Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3374
File: shesha-reactjs/src/generic-pages/settings/dynamic-theme/textsExample.tsx:13-15
Timestamp: 2025-06-10T11:52:51.462Z
Learning: In the Shesha framework's dynamic theme system, the "Global text" Typography element in textsExample.tsx is intentionally left without explicit styling to demonstrate automatic global text color application through the ConfigProvider's colorText token, while the "Default text" element uses explicit theme color styling to show the difference between global and default text color approaches.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-06-12T16:55:57.638Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3397
File: shesha-reactjs/src/designer-components/charts/bar.tsx:49-52
Timestamp: 2025-06-12T16:55:57.638Z
Learning: For the chart components’ migrators (e.g., BarChartComponent in shesha-reactjs/src/designer-components/charts/bar.tsx), the version 5 step intentionally spreads `{ ...prev, ...defaultConfigFiller }` so that values from `defaultConfigFiller` override any existing properties in `prev`. This reset to new defaults is by design and should not be flagged as an issue.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-06-26T20:01:48.838Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3461
File: shesha-reactjs/src/designer-components/charts/settingsFormIndividual.ts:171-192
Timestamp: 2025-06-26T20:01:48.838Z
Learning: In shesha-reactjs chart settings forms, duplicate input configurations may be intentionally included as part of UI/UX experiments and should be flagged for review during UI/UX review phases rather than immediately removed as errors.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-06-26T19:59:22.872Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3461
File: shesha-reactjs/src/designer-components/charts/settingsFormIndividual.ts:1280-1280
Timestamp: 2025-06-26T19:59:22.872Z
Learning: In shesha-reactjs chart settings forms, the styling panels (border, shadow, background) should not include buttonType-based hidden conditions since charts are not button components. The border configuration should follow the standard component pattern without button-specific restrictions.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2026-01-08T11:22:06.111Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4378
File: shesha-reactjs/src/designer-components/dataTable/advancedFilterButton/advancedFilterButtonComponent.tsx:61-61
Timestamp: 2026-01-08T11:22:06.111Z
Learning: In shesha-reactjs designer components (IToolboxComponent pattern), migrator functions using `.add<SpecificProps>(version, (prev) => ...)` require explicit type assertions when calling helper functions like migrateReadOnly, even though the generic parameter is specified. For example: `.add<IAdvancedFilterButtonComponentProps>(3, (prev) => migrateReadOnly(prev as IAdvancedFilterButtonComponentProps, 'inherited'))` where the cast `prev as IAdvancedFilterButtonComponentProps` must be kept.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-11-05T07:46:22.081Z
Learnt from: Lihlu
Repo: shesha-io/shesha-framework PR: 4132
File: shesha-reactjs/src/components/layerEditor/modal.tsx:55-57
Timestamp: 2025-11-05T07:46:22.081Z
Learning: In shesha-reactjs layer editor modal (src/components/layerEditor/modal.tsx), the useDeepCompareEffect that syncs items to the parent via onChange should only depend on [items], not [items, onChange]. Adding onChange to the dependency array causes the effect to fire on every parent render and breaks functionality. The guard for optional onChange is needed but the function reference should not be in the dependency array.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-11-20T14:51:46.152Z
Learnt from: AlexStepantsov
Repo: shesha-io/shesha-framework PR: 4235
File: shesha-reactjs/src/components/modelConfigurator/propertiesEditor/provider/reducer.ts:149-149
Timestamp: 2025-11-20T14:51:46.152Z
Learning: In shesha-reactjs model configurator reducer (src/components/modelConfigurator/propertiesEditor/provider/reducer.ts), when handling ArrayFormats.childObjects in the UpdateItem action, itemsType.dataFormat should be set to undefined (not ObjectFormats.object). This is the correct logic for child object array configuration.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-10-28T14:21:55.782Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4084
File: shesha-reactjs/src/components/quickView/styles/styles.ts:41-94
Timestamp: 2025-10-28T14:21:55.782Z
Learning: In shesha-reactjs/src/components/quickView/styles/styles.ts, the quickViewContent styles intentionally use multiple `!important` flags and broad selectors (including spans, form items, inputs, links, and labels) to ensure proper targeting and prevent layout breakage in the quick view component.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-05-28T07:55:21.036Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3312
File: shesha-reactjs/src/hooks/formComponentHooks.ts:183-192
Timestamp: 2025-05-28T07:55:21.036Z
Learning: In shesha-reactjs formComponentHooks.ts, the background style initialization deliberately sets valid values from properties panel immediately to provide better UX, rather than waiting for the useDeepCompareEffect to run. This intentional pattern prevents a flash of no background and shows the background image immediately when the component renders.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2026-01-14T18:34:34.784Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4408
File: shesha-reactjs/src/components/formDesigner/formComponent.tsx:115-115
Timestamp: 2026-01-14T18:34:34.784Z
Learning: In shesha-reactjs/src/components/formDesigner/formComponent.tsx, the memoized validation logic includes actualModel in the dependency array alongside toolboxComponent, getValidation, and errorCount. Since actualModel is mutated after creation from useActualContextData, including it in the dependency array should not be flagged as a memoization issue. Treat this pattern as acceptable for this file; do not raise memoization-warn checks here.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build-attempt
🔇 Additional comments (1)
shesha-reactjs/src/providers/theme/index.tsx (1)

90-95: LGTM! Previous issue successfully addressed.

The components configuration now correctly merges themeConfig.components with the new Tabs configuration, preserving the Menu.itemHeight setting defined earlier. This properly resolves the previous review concern about Menu configuration being overwritten.

✏️ Tip: You can disable this entire section by setting review_details to false in your review 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.

❤️ Share

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@shesha-reactjs/src/providers/canvas/utils.ts`:
- Around line 96-101: The code assumes sizes is non-empty but its type sizes?:
number[] allows an explicit empty array; update the function/type to prevent or
guard against empty arrays: either change the parameter type from sizes?:
number[] to a non-empty tuple like sizes: [number, ...number[]] (using
DEFAULT_OPTIONS.sizes as the fallback) or add a runtime guard before computing
mainAreaIndex/availableWidthPercent to default to DEFAULT_OPTIONS.sizes when
sizes.length === 0; reference the sizes variable, mainAreaIndex calculation,
availableWidthPercent assignment, and DEFAULT_OPTIONS.sizes when making the
change.

In `@shesha-reactjs/src/providers/theme/index.tsx`:
- Around line 87-95: The theme override replaces themeConfig.components entirely
and drops existing Menu settings; change the components assignment in the theme
prop to merge with existing components by using components: {
...themeConfig.components, Tabs: { zIndexPopup: 2000 } } so Menu.itemHeight (and
any other existing component config) is preserved while still overriding Tabs;
reference themeConfig and the Tabs entry when applying this merge.
📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ab013cf and 616aaf6.

📒 Files selected for processing (4)
  • shesha-reactjs/src/components/mainLayout/styles/indexStyles.ts
  • shesha-reactjs/src/designer-components/propertiesTabs/style.ts
  • shesha-reactjs/src/providers/canvas/utils.ts
  • shesha-reactjs/src/providers/theme/index.tsx
💤 Files with no reviewable changes (1)
  • shesha-reactjs/src/components/mainLayout/styles/indexStyles.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CodeRabbitReview-Project-Specific-Guidelines.md)

**/*.{ts,tsx}: Eliminate the any type; use unknown type instead for values with unknown types, forcing explicit type checking
Prefer type guards over type casting for type checking
Avoid monolithic types; use discriminated unions with a discriminator property instead
Leverage TypeScript to its full potential as a type system, not merely as a linter

Files:

  • shesha-reactjs/src/providers/theme/index.tsx
  • shesha-reactjs/src/providers/canvas/utils.ts
  • shesha-reactjs/src/designer-components/propertiesTabs/style.ts
🧠 Learnings (16)
📓 Common learnings
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4084
File: shesha-reactjs/src/components/quickView/styles/styles.ts:41-94
Timestamp: 2025-10-28T14:21:55.782Z
Learning: In shesha-reactjs/src/components/quickView/styles/styles.ts, the quickViewContent styles intentionally use multiple `!important` flags and broad selectors (including spans, form items, inputs, links, and labels) to ensure proper targeting and prevent layout breakage in the quick view component.
📚 Learning: 2025-10-02T11:25:33.370Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3926
File: shesha-reactjs/src/designer-components/dataTable/tableContext/styles.ts:138-166
Timestamp: 2025-10-02T11:25:33.370Z
Learning: In the shesha-framework repository, the hint popover components (sha-quick-search-hint-popover, sha-table-view-selector-hint-popover, sha-table-pager-hint-popover) in shesha-reactjs/src/designer-components/dataTable/tableContext/styles.ts intentionally use hard-coded color `rgb(214, 214, 214)` with `!important` to impose a specific consistent color across themes, overriding theme tokens by design.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
  • shesha-reactjs/src/designer-components/propertiesTabs/style.ts
📚 Learning: 2025-12-02T14:32:10.827Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4280
File: shesha-reactjs/src/designer-components/dataTable/tableContext/styles.ts:205-227
Timestamp: 2025-12-02T14:32:10.827Z
Learning: In shesha-reactjs/src/designer-components/dataTable/tableContext/styles.ts, the mockup components (filterButtonMockup and viewSelectorMockup) intentionally use hardcoded colors (`#d9d9d9`, `#fafafa`, `#8c8c8c`) to maintain consistent appearance across themes, overriding theme tokens by design.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
  • shesha-reactjs/src/designer-components/propertiesTabs/style.ts
📚 Learning: 2025-06-10T11:52:51.462Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3374
File: shesha-reactjs/src/generic-pages/settings/dynamic-theme/textsExample.tsx:13-15
Timestamp: 2025-06-10T11:52:51.462Z
Learning: In the Shesha framework's dynamic theme system, the "Global text" Typography element in textsExample.tsx is intentionally left without explicit styling to demonstrate automatic global text color application through the ConfigProvider's colorText token, while the "Default text" element uses explicit theme color styling to show the difference between global and default text color approaches.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-11-05T08:12:08.149Z
Learnt from: Lihlu
Repo: shesha-io/shesha-framework PR: 4132
File: shesha-reactjs/src/providers/layersProvider/index.tsx:35-39
Timestamp: 2025-11-05T08:12:08.149Z
Learning: In shesha-reactjs LayerGroupConfiguratorProvider (src/providers/layersProvider/index.tsx), the provider uses a unidirectional data flow pattern after initialization: props.items provide initial state to the reducer (line 35-38), user actions modify the reducer state internally, and changes sync back to the parent via onChange callbacks in consuming components (e.g., modal.tsx line 55-57 uses useDeepCompareEffect). Do NOT add a useEffect to sync props.items back to the reducer state, as this creates a feedback loop that breaks layer updates in the Calendar UI. The pattern is intentionally "controlled initialization, uncontrolled updates" where the provider manages state internally after the initial mount.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-06-12T16:55:57.638Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3397
File: shesha-reactjs/src/designer-components/charts/bar.tsx:49-52
Timestamp: 2025-06-12T16:55:57.638Z
Learning: For the chart components’ migrators (e.g., BarChartComponent in shesha-reactjs/src/designer-components/charts/bar.tsx), the version 5 step intentionally spreads `{ ...prev, ...defaultConfigFiller }` so that values from `defaultConfigFiller` override any existing properties in `prev`. This reset to new defaults is by design and should not be flagged as an issue.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
  • shesha-reactjs/src/designer-components/propertiesTabs/style.ts
📚 Learning: 2025-06-26T20:01:48.838Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3461
File: shesha-reactjs/src/designer-components/charts/settingsFormIndividual.ts:171-192
Timestamp: 2025-06-26T20:01:48.838Z
Learning: In shesha-reactjs chart settings forms, duplicate input configurations may be intentionally included as part of UI/UX experiments and should be flagged for review during UI/UX review phases rather than immediately removed as errors.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-05-28T07:55:21.036Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3312
File: shesha-reactjs/src/hooks/formComponentHooks.ts:183-192
Timestamp: 2025-05-28T07:55:21.036Z
Learning: In shesha-reactjs formComponentHooks.ts, the background style initialization deliberately sets valid values from properties panel immediately to provide better UX, rather than waiting for the useDeepCompareEffect to run. This intentional pattern prevents a flash of no background and shows the background image immediately when the component renders.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
  • shesha-reactjs/src/designer-components/propertiesTabs/style.ts
📚 Learning: 2025-06-26T19:59:22.872Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3461
File: shesha-reactjs/src/designer-components/charts/settingsFormIndividual.ts:1280-1280
Timestamp: 2025-06-26T19:59:22.872Z
Learning: In shesha-reactjs chart settings forms, the styling panels (border, shadow, background) should not include buttonType-based hidden conditions since charts are not button components. The border configuration should follow the standard component pattern without button-specific restrictions.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
  • shesha-reactjs/src/designer-components/propertiesTabs/style.ts
📚 Learning: 2025-10-28T14:21:55.782Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4084
File: shesha-reactjs/src/components/quickView/styles/styles.ts:41-94
Timestamp: 2025-10-28T14:21:55.782Z
Learning: In shesha-reactjs/src/components/quickView/styles/styles.ts, the quickViewContent styles intentionally use multiple `!important` flags and broad selectors (including spans, form items, inputs, links, and labels) to ensure proper targeting and prevent layout breakage in the quick view component.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
  • shesha-reactjs/src/designer-components/propertiesTabs/style.ts
📚 Learning: 2026-01-14T18:34:34.784Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4408
File: shesha-reactjs/src/components/formDesigner/formComponent.tsx:115-115
Timestamp: 2026-01-14T18:34:34.784Z
Learning: In shesha-reactjs/src/components/formDesigner/formComponent.tsx, the memoized validation logic includes actualModel in the dependency array alongside toolboxComponent, getValidation, and errorCount. Since actualModel is mutated after creation from useActualContextData, including it in the dependency array should not be flagged as a memoization issue. Treat this pattern as acceptable for this file; do not raise memoization-warn checks here.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-11-20T14:51:46.152Z
Learnt from: AlexStepantsov
Repo: shesha-io/shesha-framework PR: 4235
File: shesha-reactjs/src/components/modelConfigurator/propertiesEditor/provider/reducer.ts:149-149
Timestamp: 2025-11-20T14:51:46.152Z
Learning: In shesha-reactjs model configurator reducer (src/components/modelConfigurator/propertiesEditor/provider/reducer.ts), when handling ArrayFormats.childObjects in the UpdateItem action, itemsType.dataFormat should be set to undefined (not ObjectFormats.object). This is the correct logic for child object array configuration.

Applied to files:

  • shesha-reactjs/src/designer-components/propertiesTabs/style.ts
📚 Learning: 2026-01-08T11:22:06.111Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4378
File: shesha-reactjs/src/designer-components/dataTable/advancedFilterButton/advancedFilterButtonComponent.tsx:61-61
Timestamp: 2026-01-08T11:22:06.111Z
Learning: In shesha-reactjs designer components (IToolboxComponent pattern), migrator functions using `.add<SpecificProps>(version, (prev) => ...)` require explicit type assertions when calling helper functions like migrateReadOnly, even though the generic parameter is specified. For example: `.add<IAdvancedFilterButtonComponentProps>(3, (prev) => migrateReadOnly(prev as IAdvancedFilterButtonComponentProps, 'inherited'))` where the cast `prev as IAdvancedFilterButtonComponentProps` must be kept.

Applied to files:

  • shesha-reactjs/src/designer-components/propertiesTabs/style.ts
📚 Learning: 2026-01-08T13:24:42.323Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4408
File: shesha-reactjs/src/components/formDesigner/formComponent.tsx:152-159
Timestamp: 2026-01-08T13:24:42.323Z
Learning: In shesha-reactjs form designer validation (shesha-reactjs/src/components/formDesigner/formComponent.tsx), the use of JSON.stringify(store.modelType) in the datatable columns mismatch error message is intentional and should not be flagged as redundant.

Applied to files:

  • shesha-reactjs/src/designer-components/propertiesTabs/style.ts
📚 Learning: 2026-01-08T11:19:34.113Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4378
File: shesha-reactjs/src/designer-components/dataTable/advancedFilterButton/advancedFilterButtonComponent.tsx:48-48
Timestamp: 2026-01-08T11:19:34.113Z
Learning: In shesha-reactjs designer components (IToolboxComponent pattern), the Factory function receives a model parameter typed as the base IConfigurableFormComponent. When a component uses IToolboxComponent<SpecificProps>, a type assertion (e.g., `model as SpecificProps`) is necessary when passing model to the component to narrow from the base type to the specific prop type, even though the generic parameter is specified. This is required when SpecificProps has required properties not present in IConfigurableFormComponent.

Applied to files:

  • shesha-reactjs/src/designer-components/propertiesTabs/style.ts
📚 Learning: 2025-12-11T11:57:48.398Z
Learnt from: czwe-01
Repo: shesha-io/shesha-framework PR: 4349
File: shesha-reactjs/src/designer-components/columns/styles.ts:3-14
Timestamp: 2025-12-11T11:57:48.398Z
Learning: In the shesha-framework's shesha-reactjs project, when using createStyles to define style hooks (e.g., export const useStyles = createStyles(...)), return an object from the callback with named keys (e.g., { columns }) and access them in components as const { styles } = useStyles(); then read properties via styles.columns. The wrapping behavior is provided by createStyles and should be followed consistently. This pattern applies to style modules under shesha-reactjs/src/designer-components (and similar styling files).

Applied to files:

  • shesha-reactjs/src/designer-components/propertiesTabs/style.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build-attempt
🔇 Additional comments (1)
shesha-reactjs/src/designer-components/propertiesTabs/style.ts (1)

24-35: LGTM with a minor note on CSS value choice.

The addition of justify-content: normal explicitly resets the main-axis alignment. For flex containers, normal behaves as start. While valid, using flex-start or start would be more explicit and self-documenting. This is a minor stylistic preference and doesn't affect functionality.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

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/providers/theme/index.tsx (1)

67-71: Remove unnecessary any type cast from Menu itemHeight.

Line 69 unnecessarily uses as any to cast a CSS clamp string. Ant Design's Menu component itemHeight property accepts number | string, so the CSS clamp string is already a valid type. Simply remove the cast:

itemHeight: 'clamp(40px, 40px, 100%)',

This eliminates the violation of the coding guideline: "Eliminate the any type; use unknown type instead for values with unknown types, forcing explicit type checking."

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 77e59dd and 551a496.

📒 Files selected for processing (1)
  • shesha-reactjs/src/providers/theme/index.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CodeRabbitReview-Project-Specific-Guidelines.md)

**/*.{ts,tsx}: Eliminate the any type; use unknown type instead for values with unknown types, forcing explicit type checking
Prefer type guards over type casting for type checking
Avoid monolithic types; use discriminated unions with a discriminator property instead
Leverage TypeScript to its full potential as a type system, not merely as a linter

Files:

  • shesha-reactjs/src/providers/theme/index.tsx
🧠 Learnings (14)
📓 Common learnings
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4084
File: shesha-reactjs/src/components/quickView/styles/styles.ts:41-94
Timestamp: 2025-10-28T14:21:55.782Z
Learning: In shesha-reactjs/src/components/quickView/styles/styles.ts, the quickViewContent styles intentionally use multiple `!important` flags and broad selectors (including spans, form items, inputs, links, and labels) to ensure proper targeting and prevent layout breakage in the quick view component.
📚 Learning: 2025-10-02T11:25:33.370Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3926
File: shesha-reactjs/src/designer-components/dataTable/tableContext/styles.ts:138-166
Timestamp: 2025-10-02T11:25:33.370Z
Learning: In the shesha-framework repository, the hint popover components (sha-quick-search-hint-popover, sha-table-view-selector-hint-popover, sha-table-pager-hint-popover) in shesha-reactjs/src/designer-components/dataTable/tableContext/styles.ts intentionally use hard-coded color `rgb(214, 214, 214)` with `!important` to impose a specific consistent color across themes, overriding theme tokens by design.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-11-05T08:12:08.149Z
Learnt from: Lihlu
Repo: shesha-io/shesha-framework PR: 4132
File: shesha-reactjs/src/providers/layersProvider/index.tsx:35-39
Timestamp: 2025-11-05T08:12:08.149Z
Learning: In shesha-reactjs LayerGroupConfiguratorProvider (src/providers/layersProvider/index.tsx), the provider uses a unidirectional data flow pattern after initialization: props.items provide initial state to the reducer (line 35-38), user actions modify the reducer state internally, and changes sync back to the parent via onChange callbacks in consuming components (e.g., modal.tsx line 55-57 uses useDeepCompareEffect). Do NOT add a useEffect to sync props.items back to the reducer state, as this creates a feedback loop that breaks layer updates in the Calendar UI. The pattern is intentionally "controlled initialization, uncontrolled updates" where the provider manages state internally after the initial mount.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-12-02T14:32:10.827Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4280
File: shesha-reactjs/src/designer-components/dataTable/tableContext/styles.ts:205-227
Timestamp: 2025-12-02T14:32:10.827Z
Learning: In shesha-reactjs/src/designer-components/dataTable/tableContext/styles.ts, the mockup components (filterButtonMockup and viewSelectorMockup) intentionally use hardcoded colors (`#d9d9d9`, `#fafafa`, `#8c8c8c`) to maintain consistent appearance across themes, overriding theme tokens by design.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-06-10T11:52:51.462Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3374
File: shesha-reactjs/src/generic-pages/settings/dynamic-theme/textsExample.tsx:13-15
Timestamp: 2025-06-10T11:52:51.462Z
Learning: In the Shesha framework's dynamic theme system, the "Global text" Typography element in textsExample.tsx is intentionally left without explicit styling to demonstrate automatic global text color application through the ConfigProvider's colorText token, while the "Default text" element uses explicit theme color styling to show the difference between global and default text color approaches.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-06-12T16:55:57.638Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3397
File: shesha-reactjs/src/designer-components/charts/bar.tsx:49-52
Timestamp: 2025-06-12T16:55:57.638Z
Learning: For the chart components’ migrators (e.g., BarChartComponent in shesha-reactjs/src/designer-components/charts/bar.tsx), the version 5 step intentionally spreads `{ ...prev, ...defaultConfigFiller }` so that values from `defaultConfigFiller` override any existing properties in `prev`. This reset to new defaults is by design and should not be flagged as an issue.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-06-26T20:01:48.838Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3461
File: shesha-reactjs/src/designer-components/charts/settingsFormIndividual.ts:171-192
Timestamp: 2025-06-26T20:01:48.838Z
Learning: In shesha-reactjs chart settings forms, duplicate input configurations may be intentionally included as part of UI/UX experiments and should be flagged for review during UI/UX review phases rather than immediately removed as errors.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-06-26T19:59:22.872Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3461
File: shesha-reactjs/src/designer-components/charts/settingsFormIndividual.ts:1280-1280
Timestamp: 2025-06-26T19:59:22.872Z
Learning: In shesha-reactjs chart settings forms, the styling panels (border, shadow, background) should not include buttonType-based hidden conditions since charts are not button components. The border configuration should follow the standard component pattern without button-specific restrictions.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2026-01-08T11:22:06.111Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4378
File: shesha-reactjs/src/designer-components/dataTable/advancedFilterButton/advancedFilterButtonComponent.tsx:61-61
Timestamp: 2026-01-08T11:22:06.111Z
Learning: In shesha-reactjs designer components (IToolboxComponent pattern), migrator functions using `.add<SpecificProps>(version, (prev) => ...)` require explicit type assertions when calling helper functions like migrateReadOnly, even though the generic parameter is specified. For example: `.add<IAdvancedFilterButtonComponentProps>(3, (prev) => migrateReadOnly(prev as IAdvancedFilterButtonComponentProps, 'inherited'))` where the cast `prev as IAdvancedFilterButtonComponentProps` must be kept.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-11-05T07:46:22.081Z
Learnt from: Lihlu
Repo: shesha-io/shesha-framework PR: 4132
File: shesha-reactjs/src/components/layerEditor/modal.tsx:55-57
Timestamp: 2025-11-05T07:46:22.081Z
Learning: In shesha-reactjs layer editor modal (src/components/layerEditor/modal.tsx), the useDeepCompareEffect that syncs items to the parent via onChange should only depend on [items], not [items, onChange]. Adding onChange to the dependency array causes the effect to fire on every parent render and breaks functionality. The guard for optional onChange is needed but the function reference should not be in the dependency array.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-11-20T14:51:46.152Z
Learnt from: AlexStepantsov
Repo: shesha-io/shesha-framework PR: 4235
File: shesha-reactjs/src/components/modelConfigurator/propertiesEditor/provider/reducer.ts:149-149
Timestamp: 2025-11-20T14:51:46.152Z
Learning: In shesha-reactjs model configurator reducer (src/components/modelConfigurator/propertiesEditor/provider/reducer.ts), when handling ArrayFormats.childObjects in the UpdateItem action, itemsType.dataFormat should be set to undefined (not ObjectFormats.object). This is the correct logic for child object array configuration.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-10-28T14:21:55.782Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4084
File: shesha-reactjs/src/components/quickView/styles/styles.ts:41-94
Timestamp: 2025-10-28T14:21:55.782Z
Learning: In shesha-reactjs/src/components/quickView/styles/styles.ts, the quickViewContent styles intentionally use multiple `!important` flags and broad selectors (including spans, form items, inputs, links, and labels) to ensure proper targeting and prevent layout breakage in the quick view component.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2025-05-28T07:55:21.036Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3312
File: shesha-reactjs/src/hooks/formComponentHooks.ts:183-192
Timestamp: 2025-05-28T07:55:21.036Z
Learning: In shesha-reactjs formComponentHooks.ts, the background style initialization deliberately sets valid values from properties panel immediately to provide better UX, rather than waiting for the useDeepCompareEffect to run. This intentional pattern prevents a flash of no background and shows the background image immediately when the component renders.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
📚 Learning: 2026-01-14T18:34:34.784Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4408
File: shesha-reactjs/src/components/formDesigner/formComponent.tsx:115-115
Timestamp: 2026-01-14T18:34:34.784Z
Learning: In shesha-reactjs/src/components/formDesigner/formComponent.tsx, the memoized validation logic includes actualModel in the dependency array alongside toolboxComponent, getValidation, and errorCount. Since actualModel is mutated after creation from useActualContextData, including it in the dependency array should not be flagged as a memoization issue. Treat this pattern as acceptable for this file; do not raise memoization-warn checks here.

Applied to files:

  • shesha-reactjs/src/providers/theme/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build-attempt
🔇 Additional comments (1)
shesha-reactjs/src/providers/theme/index.tsx (1)

90-95: LGTM! Previous issue successfully addressed.

The components configuration now correctly merges themeConfig.components with the new Tabs configuration, preserving the Menu.itemHeight setting defined earlier. This properly resolves the previous review concern about Menu configuration being overwritten.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

@czwe-01 czwe-01 marked this pull request as ready for review January 15, 2026 13:18
@czwe-01 czwe-01 requested a review from James-Baloyi January 15, 2026 13:18
@James-Baloyi James-Baloyi merged commit b334b5a into shesha-io:main Jan 21, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants