Merged
Conversation
…anvas styles for better overflow handling
Contributor
WalkthroughReplaced hardcoded sidebar/tool heights and widths with named constants, applied consistent thin-scrollbar styling across multiple containers, removed several fixed-height/min-height rules, deleted drag-and-drop CSS in form designer, and removed an inline bottom padding from the component properties panel. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (9)📓 Common learnings📚 Learning: 2025-10-28T14:21:55.782ZApplied to files:
📚 Learning: 2025-06-26T19:59:22.872ZApplied to files:
📚 Learning: 2025-05-28T07:55:21.036ZApplied to files:
📚 Learning: 2025-06-12T16:55:57.638ZApplied to files:
📚 Learning: 2025-10-02T11:25:33.370ZApplied to files:
📚 Learning: 2025-11-05T07:46:22.081ZApplied to files:
📚 Learning: 2025-06-10T11:52:51.462ZApplied to files:
📚 Learning: 2025-07-19T10:27:44.987ZApplied to files:
⏰ 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)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
shesha-reactjs/src/components/formDesigner/componentPropertiesPanel/index.tsx(1 hunks)shesha-reactjs/src/components/sidebarContainer/styles/styles.ts(4 hunks)shesha-reactjs/src/configuration-studio/styles.ts(3 hunks)shesha-reactjs/src/designer-components/_settings/styles/styles.ts(0 hunks)
💤 Files with no reviewable changes (1)
- shesha-reactjs/src/designer-components/_settings/styles/styles.ts
🧰 Additional context used
🧠 Learnings (9)
📓 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-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/components/sidebarContainer/styles/styles.tsshesha-reactjs/src/configuration-studio/styles.tsshesha-reactjs/src/components/formDesigner/componentPropertiesPanel/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/components/sidebarContainer/styles/styles.tsshesha-reactjs/src/components/formDesigner/componentPropertiesPanel/index.tsx
📚 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/configuration-studio/styles.ts
📚 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/configuration-studio/styles.tsshesha-reactjs/src/components/formDesigner/componentPropertiesPanel/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/components/formDesigner/componentPropertiesPanel/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/components/formDesigner/componentPropertiesPanel/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/components/formDesigner/componentPropertiesPanel/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/components/formDesigner/componentPropertiesPanel/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 (5)
shesha-reactjs/src/components/formDesigner/componentPropertiesPanel/index.tsx (1)
18-18: LGTM! Layout adjustment aligns with broader spacing changes.Removing the inline padding delegates spacing control to parent containers, which is consistent with the layout adjustments made across the sidebar and configuration studio components in this PR.
shesha-reactjs/src/components/sidebarContainer/styles/styles.ts (2)
129-129: Verify the min-height adjustment doesn't cause layout issues.The min-height offset increased by 48px (from
102pxto150px), which is a significant adjustment. Please verify this change doesn't cause unexpected layout issues or overflow behavior, especially on smaller screens or when sidebars contain substantial content.Also applies to: 140-140
171-171: LGTM! Correct use ofoverflow: auto.The
overflow: autoproperty correctly shows scrollbars only when content overflows, providing good UX.shesha-reactjs/src/configuration-studio/styles.ts (2)
102-104: LGTM! Height management aligns with PR layout objectives.The
height: 100%rule forcsMainAreawithin thecsDocEditorscope ensures the main area fills available space, which is consistent with the height and overflow adjustments made across the sidebar and canvas components in this PR.
21-21: csMainArea constant is correctly not exported.Verification confirms
csMainAreaand its class name"sha-cs-main-area"have zero external references across the codebase. The constant is used solely for internal CSS scoping within styles.ts (line 102), which is an appropriate use case where export is unnecessary.
shesha-reactjs/src/components/sidebarContainer/styles/styles.ts
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
shesha-reactjs/src/components/sidebarContainer/styles/styles.ts (1)
53-54: LGTM! Overflow behavior improved.The
overflow: autoresolves the previous concern about forced scrollbars. Scrollbars will now appear only when content exceeds the container height.
🧹 Nitpick comments (1)
shesha-reactjs/src/components/sidebarContainer/styles/styles.ts (1)
53-54: Consider extracting repeated height calculations into constants.The viewport height calculations
calc(100vh - 120px)andcalc(100vh - 150px)are repeated across multiple locations. Extracting these as named constants would improve maintainability and clarify what the magic numbers represent.Example refactor:
export const useStyles = createStyles(({ css, cx, prefixCls }) => { const leftSidebarWidth = "550px"; const sidebarBtnHeight = "35px"; + const containerHeight = "calc(100vh - 120px)"; + const minContainerHeight = "calc(100vh - 150px)";Then reference these constants throughout the styles.
Also applies to: 170-171
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
shesha-reactjs/src/components/sidebarContainer/styles/styles.ts(4 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
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.
📚 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/components/sidebarContainer/styles/styles.ts
📚 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/components/sidebarContainer/styles/styles.ts
📚 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/components/sidebarContainer/styles/styles.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 (2)
shesha-reactjs/src/components/sidebarContainer/styles/styles.ts (2)
113-113: Verify fixed85vhheight interacts correctly with parent container heights.The
.sidebar-body-content.openuses a fixed85vhheight, while its ancestors use calculated heights likecalc(100vh - 120px)(line 54). This mixing of fixed viewport-based heights with calculated heights can cause unexpected scrolling behavior or content overflow. Verify that the sidebar content displays correctly across different viewport sizes and doesn't create nested scrollbars or clipped content.
129-129: Manual verification required: Confirm the min-height reduction doesn't cause layout issues on smaller viewports.The scripts confirm that lines 129 and 140 use
calc(100vh - 150px)for bothsidebarContainerLeftandsidebarContainerRight, and line 54 usescalc(100vh - 120px)for thesidebarBody. However, the search results don't show the original "102px" value in the current codebase, making it impossible to verify the exact reduction amount or the reason for the change.Please confirm:
- The previous value was indeed 102px (check git history)
- Testing was performed on viewports ≤ 600px height to ensure no content clipping
- The 150px offset correctly represents the combined header/toolbar/footer heights
IvanIlyichev
requested changes
Nov 10, 2025
…ha-framework into thulasizwe/fix/3558
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
shesha-reactjs/src/components/formDesigner/styles/styles.ts (1)
147-155: Full-height layout approach looks good.The
height: 100%rules for the form designer, main area, and sidebar container should establish the full-height layout correctly.Minor style note: Line 155 ends with
};(semicolon after the closing brace), which is syntactically valid but stylistically inconsistent with the rest of the file where CSS blocks close without a trailing semicolon.Consider removing the trailing semicolon for consistency:
.sidebar-container { height: 100%; } - }; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
shesha-reactjs/src/components/formDesigner/styles/styles.ts(2 hunks)shesha-reactjs/src/configuration-studio/styles.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- shesha-reactjs/src/configuration-studio/styles.ts
🧰 Additional context used
🧠 Learnings (5)
📚 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/components/formDesigner/styles/styles.ts
📚 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/components/formDesigner/styles/styles.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/components/formDesigner/styles/styles.ts
📚 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/components/formDesigner/styles/styles.ts
📚 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/components/formDesigner/styles/styles.ts
🔇 Additional comments (1)
shesha-reactjs/src/components/formDesigner/styles/styles.ts (1)
93-93: LGTM! Enabling mainArea usage.Good addition—the
mainAreaclass name was defined but previously unused, and is now properly destructured to support the full-height layout rules below.
…r, sidebar, and configuration studio components
Contributor
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/sidebarContainer/styles/styles.ts (1)
207-247: Note: Duplicated scrollbar styling within the same file.The scrollbar styling block is repeated nearly identically for
.canvasWrapper(lines 210-223) and[data-sha-c-type="datatable"] .canvasWrapper(lines 232-245). While the datatable-specific selector might require separate styling for specificity, consider whether the duplication can be reduced.If both need identical scrollbar styling, you could:
- Extract the scrollbar styles to a shared mixin (as suggested in formDesigner/styles/styles.ts)
- Or use a comma-separated selector if the specificity requirements allow it
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
shesha-reactjs/src/components/formDesigner/styles/styles.ts(2 hunks)shesha-reactjs/src/components/sidebarContainer/styles/styles.ts(5 hunks)shesha-reactjs/src/configuration-studio/styles.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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.
📚 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/configuration-studio/styles.tsshesha-reactjs/src/components/sidebarContainer/styles/styles.tsshesha-reactjs/src/components/formDesigner/styles/styles.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/configuration-studio/styles.tsshesha-reactjs/src/components/sidebarContainer/styles/styles.tsshesha-reactjs/src/components/formDesigner/styles/styles.ts
📚 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/components/sidebarContainer/styles/styles.tsshesha-reactjs/src/components/formDesigner/styles/styles.ts
📚 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/components/sidebarContainer/styles/styles.tsshesha-reactjs/src/components/formDesigner/styles/styles.ts
📚 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/components/sidebarContainer/styles/styles.ts
📚 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/components/formDesigner/styles/styles.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). (2)
- GitHub Check: build-attempt
- GitHub Check: build-attempt
🔇 Additional comments (3)
shesha-reactjs/src/components/sidebarContainer/styles/styles.ts (1)
63-80: LGTM! Overflow behavior correctly updated.The change from
overflow: scrolltooverflow: autoimproves UX by only showing scrollbars when content overflows, as requested in the previous review.shesha-reactjs/src/configuration-studio/styles.ts (2)
88-91: LGTM! Overflow behavior correctly delegated to nested elements.Changing the work area overflow to
hiddenis appropriate since scrolling is now handled by the nested doc tabs content holder (lines 95-109). This provides better control over the scrolling behavior.
21-21: Verify that thecsMainAreaclass is applied to DOM elements.A past reviewer noted they couldn't find any usages of the
sha-cs-main-areaclass in the application. While the class is defined and used in the styles, it needs to be applied to actual DOM elements in the component files to take effect.Run this script to verify whether the class is applied in component files:
If the class is not applied to any DOM elements (via className prop), the constant and styling should be removed as dead code.
Also applies to: 120-122
…iguration studio components
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
shesha-reactjs/src/components/formDesigner/styles/styles.ts (1)
378-380: Consider extracting hardcoded height values to named constants.The hardcoded pixel values (60px for header height and 56px for toolbar height) are fragile and difficult to maintain. If these UI elements change height, multiple locations may need updates.
Consider defining these as constants at the file level:
const HEADER_HEIGHT = 60; const TOOLBAR_HEIGHT = 56; // Then use: .${designerClassNames.mainArea}{ height: calc(100vh - ${HEADER_HEIGHT}px - ${TOOLBAR_HEIGHT}px); }shesha-reactjs/src/configuration-studio/styles.ts (2)
59-59: Add missing WebKit scrollbar pseudo-elements for cross-browser consistency.The tree area only includes
scrollbar-width: thin;(Firefox-specific) but lacks the WebKit scrollbar pseudo-elements used elsewhere in this PR. This creates inconsistent scrollbar appearance across browsers.Apply this diff to add the missing WebKit scrollbar styling or use
${sheshaStyles.thinScrollbars}:.${csTreeArea}{ height: calc(100vh - ${headerHeight}px); overflow: 'auto'; scrollbar-width: thin; + &::-webkit-scrollbar { + width: 6px; + } + &::-webkit-scrollbar-track { + background: transparent; + } + &::-webkit-scrollbar-thumb { + background: #d3d3d3; + border-radius: 3px; + } + &::-webkit-scrollbar-thumb:hover { + background: #a0a0a0; + }Alternatively, replace
scrollbar-width: thin;with${sheshaStyles.thinScrollbars}for consistency with line 95.
94-95: Consider extracting hardcoded 56px value to a named constant.The hardcoded
56pxvalue (likely tab bar height) appears in height calculations. For consistency with theheaderHeightconstant and improved maintainability, this should be extracted.const headerHeight = 60; const tabBarHeight = 56; // Then use: height: calc(100vh - ${headerHeight}px - ${tabBarHeight}px);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
shesha-reactjs/src/components/formDesigner/styles/styles.ts(2 hunks)shesha-reactjs/src/components/sidebarContainer/styles/styles.ts(5 hunks)shesha-reactjs/src/configuration-studio/styles.ts(4 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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.
📚 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/components/formDesigner/styles/styles.tsshesha-reactjs/src/components/sidebarContainer/styles/styles.tsshesha-reactjs/src/configuration-studio/styles.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/components/formDesigner/styles/styles.tsshesha-reactjs/src/components/sidebarContainer/styles/styles.tsshesha-reactjs/src/configuration-studio/styles.ts
📚 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/components/formDesigner/styles/styles.tsshesha-reactjs/src/configuration-studio/styles.ts
📚 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/components/formDesigner/styles/styles.tsshesha-reactjs/src/components/sidebarContainer/styles/styles.tsshesha-reactjs/src/configuration-studio/styles.ts
📚 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/components/formDesigner/styles/styles.tsshesha-reactjs/src/components/sidebarContainer/styles/styles.tsshesha-reactjs/src/configuration-studio/styles.ts
📚 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/components/sidebarContainer/styles/styles.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
shesha-reactjs/src/components/sidebarContainer/styles/styles.ts
Outdated
Show resolved
Hide resolved
shesha-reactjs/src/components/sidebarContainer/styles/styles.ts
Outdated
Show resolved
Hide resolved
…lbar styles in form designer and sidebar components
…variable names and layout calculations
Collaborator
Author
|
This PR is meant to. address issue relating to Canva overlaping with property panel on manual zoom, |
IvanIlyichev
approved these changes
Nov 18, 2025
This was referenced Nov 19, 2025
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#3558
Summary by CodeRabbit