Merged
Conversation
…improved layout and alignment
Contributor
|
Warning Rate limit exceeded@czwe-01 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 28 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (1)
WalkthroughReplaced inline wrapper layout styles in ReadOnlyDisplayFormItem with a CSS-module Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
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: 4
♻️ Duplicate comments (1)
shesha-reactjs/src/components/readOnlyDisplayFormItem/index.tsx (1)
35-35: Validate textAlign values before passing to useStyles.Same issue as in inputField.tsx:
style?.textAligncan be any valid CSS text-align value, but the wrapper logic only handles 'center', 'right', and defaults to 'flex-start'. Unexpected values could result in unintended layout behavior.Apply the same validation as suggested for inputField.tsx:
- const { styles } = useStyles({ textAlign: style?.textAlign || 'left' }); + const normalizedTextAlign = style?.textAlign === 'center' || style?.textAlign === 'right' + ? style.textAlign + : 'left'; + const { styles } = useStyles({ textAlign: normalizedTextAlign });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
shesha-reactjs/src/components/readOnlyDisplayFormItem/index.tsx(1 hunks)shesha-reactjs/src/components/readOnlyDisplayFormItem/inputField.tsx(1 hunks)shesha-reactjs/src/components/readOnlyDisplayFormItem/styles/styles.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CodeRabbitReview-Project-Specific-Guidelines.md)
**/*.{ts,tsx}: Eliminate theanytype; useunknowntype 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/components/readOnlyDisplayFormItem/styles/styles.tsshesha-reactjs/src/components/readOnlyDisplayFormItem/index.tsxshesha-reactjs/src/components/readOnlyDisplayFormItem/inputField.tsx
🧠 Learnings (12)
📓 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.
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.
📚 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/readOnlyDisplayFormItem/styles/styles.tsshesha-reactjs/src/components/readOnlyDisplayFormItem/index.tsxshesha-reactjs/src/components/readOnlyDisplayFormItem/inputField.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/components/readOnlyDisplayFormItem/styles/styles.tsshesha-reactjs/src/components/readOnlyDisplayFormItem/index.tsxshesha-reactjs/src/components/readOnlyDisplayFormItem/inputField.tsx
📚 Learning: 2025-12-02T14:27:56.659Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4280
File: shesha-reactjs/src/components/tablePager/index.tsx:49-55
Timestamp: 2025-12-02T14:27:56.659Z
Learning: In shesha-reactjs hint Popover components (tablePagerHintPopover, quickSearchHintPopover, tableViewSelectorHintPopover), both `rootClassName` and `classNames.body` must be set to the same style class for the styles to be applied correctly to the different DOM nodes of the Ant Design Popover component.
Applied to files:
shesha-reactjs/src/components/readOnlyDisplayFormItem/styles/styles.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/components/readOnlyDisplayFormItem/styles/styles.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/components/readOnlyDisplayFormItem/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/readOnlyDisplayFormItem/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/readOnlyDisplayFormItem/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/components/readOnlyDisplayFormItem/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/readOnlyDisplayFormItem/index.tsxshesha-reactjs/src/components/readOnlyDisplayFormItem/inputField.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/readOnlyDisplayFormItem/index.tsx
📚 Learning: 2025-11-25T07:17:44.449Z
Learnt from: CR
Repo: shesha-io/shesha-framework PR: 0
File: CodeRabbitReview-Project-Specific-Guidelines.md:0-0
Timestamp: 2025-11-25T07:17:44.449Z
Learning: Applies to **/*{Component,Editor}.{ts,tsx} : Remove the `defaultValue` pattern entirely from controlled editor components
Applied to files:
shesha-reactjs/src/components/readOnlyDisplayFormItem/inputField.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/components/readOnlyDisplayFormItem/styles/styles.ts (1)
59-67: LGTM! Wrapper class properly addresses layout concerns.The new wrapper class correctly implements flex layout with dynamic alignment based on textAlign, addressing the scroll issue mentioned in the PR objectives. The flex-wrap and box-sizing properties ensure content wraps properly without causing overflow.
shesha-reactjs/src/components/readOnlyDisplayFormItem/index.tsx
Outdated
Show resolved
Hide resolved
shesha-reactjs/src/components/readOnlyDisplayFormItem/inputField.tsx
Outdated
Show resolved
Hide resolved
shesha-reactjs/src/components/readOnlyDisplayFormItem/inputField.tsx
Outdated
Show resolved
Hide resolved
shesha-reactjs/src/components/readOnlyDisplayFormItem/styles/styles.ts
Outdated
Show resolved
Hide resolved
…ponents for better readability
…lign parameter for improved text alignment control
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
shesha-reactjs/src/components/readOnlyDisplayFormItem/styles/styles.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CodeRabbitReview-Project-Specific-Guidelines.md)
**/*.{ts,tsx}: Eliminate theanytype; useunknowntype 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/components/readOnlyDisplayFormItem/styles/styles.ts
🧠 Learnings (11)
📓 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/readOnlyDisplayFormItem/styles/styles.ts
📚 Learning: 2025-11-25T07:17:44.449Z
Learnt from: CR
Repo: shesha-io/shesha-framework PR: 0
File: CodeRabbitReview-Project-Specific-Guidelines.md:0-0
Timestamp: 2025-11-25T07:17:44.449Z
Learning: Applies to **/*.{ts,tsx} : Leverage TypeScript to its full potential as a type system, not merely as a linter
Applied to files:
shesha-reactjs/src/components/readOnlyDisplayFormItem/styles/styles.ts
📚 Learning: 2025-11-25T07:17:44.449Z
Learnt from: CR
Repo: shesha-io/shesha-framework PR: 0
File: CodeRabbitReview-Project-Specific-Guidelines.md:0-0
Timestamp: 2025-11-25T07:17:44.449Z
Learning: Applies to **/*.{ts,tsx} : Eliminate the `any` type; use `unknown` type instead for values with unknown types, forcing explicit type checking
Applied to files:
shesha-reactjs/src/components/readOnlyDisplayFormItem/styles/styles.ts
📚 Learning: 2025-11-25T07:17:44.449Z
Learnt from: CR
Repo: shesha-io/shesha-framework PR: 0
File: CodeRabbitReview-Project-Specific-Guidelines.md:0-0
Timestamp: 2025-11-25T07:17:44.449Z
Learning: Applies to **/*.{ts,tsx} : Avoid monolithic types; use discriminated unions with a discriminator property instead
Applied to files:
shesha-reactjs/src/components/readOnlyDisplayFormItem/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/readOnlyDisplayFormItem/styles/styles.ts
📚 Learning: 2025-12-02T14:27:56.659Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4280
File: shesha-reactjs/src/components/tablePager/index.tsx:49-55
Timestamp: 2025-12-02T14:27:56.659Z
Learning: In shesha-reactjs hint Popover components (tablePagerHintPopover, quickSearchHintPopover, tableViewSelectorHintPopover), both `rootClassName` and `classNames.body` must be set to the same style class for the styles to be applied correctly to the different DOM nodes of the Ant Design Popover component.
Applied to files:
shesha-reactjs/src/components/readOnlyDisplayFormItem/styles/styles.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/components/readOnlyDisplayFormItem/styles/styles.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/components/readOnlyDisplayFormItem/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/readOnlyDisplayFormItem/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/readOnlyDisplayFormItem/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 (4)
shesha-reactjs/src/components/readOnlyDisplayFormItem/styles/styles.ts (4)
2-2: LGTM!The
CSSPropertiesimport is appropriately used to type thetextAlignproperty, ensuring type safety.
8-9: LGTM!The function signature correctly implements the type-safe parameter pattern, addressing the previous review concern about implicit
anytypes. The destructuring oftextAlignfrom the typed params object is clean and idiomatic.
65-73: Well-designed solution for preventing overflow issues.The
wrapperclass effectively addresses the PR objective of preventing unwanted scrolls from readonly form items:
flex-wrap: wrapallows content to wrap instead of overflowingbox-sizing: border-boxensures accurate width calculations including padding- Dynamic
justify-contentcorrectly maps text alignment valuesThe flexbox approach is a robust solution for managing multi-item layouts without causing horizontal overflow.
79-79: LGTM!The addition of
wrapperto the return object follows the established pattern for exporting style classes.
shesha-reactjs/src/components/readOnlyDisplayFormItem/styles/styles.ts
Outdated
Show resolved
Hide resolved
…for improved type definition clarity
James-Baloyi
approved these changes
Dec 8, 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.
Addressed issue: readonly form items causing scrolls
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.