Conversation
WalkthroughReplaces span wrappers with anchors for file name blocks, adds header background size/position/repeat properties to panel styles, adjusts FileUpload style defaults and button/text styles, and broadens designer fileUpload finalStyle to also use reduced styles when listType === 'text'. Changes
Sequence Diagram(s)sequenceDiagram
participant Designer as Designer component
participant Props as Props (readOnly, enableStyleOnReadonly, listType)
participant StyleCalc as finalStyle calculation
Note over Designer,Props: On render, Designer asks for style mode
Designer->>Props: provide readOnly, enableStyleOnReadonly, listType
Props->>StyleCalc: evaluate conditions
alt (enableStyleOnReadonly == false AND readOnly == true) OR (listType == 'text')
StyleCalc-->>Designer: return reducedStyle
else
StyleCalc-->>Designer: return fullStyle
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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 (4)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
shesha-reactjs/src/components/fileUpload/index.tsx(1 hunks)shesha-reactjs/src/components/panel/styles/styles.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: teboho
PR: shesha-io/shesha-framework#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-05-28T07:55:21.036Z
Learnt from: teboho
PR: shesha-io/shesha-framework#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/panel/styles/styles.ts
📚 Learning: 2025-06-26T19:59:22.872Z
Learnt from: teboho
PR: shesha-io/shesha-framework#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/panel/styles/styles.ts
🔇 Additional comments (1)
shesha-reactjs/src/components/panel/styles/styles.ts (1)
61-66: Header background sizing update looks goodThanks for mirroring the background sizing/position/repeat options onto the panel header—this keeps the header styling in sync with the body customization knobs without introducing any regressions.
Also applies to: 164-166
| type="link" | ||
| disabled={!showUploadButton} | ||
| style={{width: '100%', height: '100%'}} | ||
| style={{width: '100%', height: '100%', justifyContent: props?.style?.textAlign || 'left'}} |
There was a problem hiding this comment.
Fix the invalid flex alignment value
Button renders an inline-flex container, but justify-content doesn’t accept 'left'. Passing 'left' (the default when textAlign is missing) results in an invalid declaration, so the button remains center-aligned instead of honoring the provided alignment. Please translate the common text-align values to their flex equivalents (left/start → flex-start, right/end → flex-end, etc.) before assigning them.
Apply something along these lines:
+ const alignMap: Record<string, React.CSSProperties['justifyContent']> = {
+ left: 'flex-start',
+ start: 'flex-start',
+ center: 'center',
+ right: 'flex-end',
+ end: 'flex-end',
+ };
+ const uploadButtonJustify = props?.style?.textAlign
+ ? alignMap[props.style.textAlign] ?? 'flex-start'
+ : 'flex-start';
+
const uploadButton = (
allowUpload && <Button
icon={!fileInfo ? <UploadOutlined /> : <PictureOutlined />}
type="link"
disabled={!showUploadButton}
- style={{width: '100%', height: '100%', justifyContent: props?.style?.textAlign || 'left'}}
+ style={{ width: '100%', height: '100%', justifyContent: uploadButtonJustify }}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| style={{width: '100%', height: '100%', justifyContent: props?.style?.textAlign || 'left'}} | |
| // Map common textAlign values to valid flex justifyContent values | |
| const alignMap: Record<string, React.CSSProperties['justifyContent']> = { | |
| left: 'flex-start', | |
| start: 'flex-start', | |
| center: 'center', | |
| right: 'flex-end', | |
| end: 'flex-end', | |
| }; | |
| // Compute the actual justifyContent value, defaulting to 'flex-start' | |
| const uploadButtonJustify = props?.style?.textAlign | |
| ? alignMap[props.style.textAlign] ?? 'flex-start' | |
| : 'flex-start'; | |
| const uploadButton = ( | |
| allowUpload && ( | |
| <Button | |
| icon={!fileInfo ? <UploadOutlined /> : <PictureOutlined />} | |
| type="link" | |
| disabled={!showUploadButton} | |
| style={{ | |
| width: '100%', | |
| height: '100%', | |
| justifyContent: uploadButtonJustify, | |
| }} | |
| > | |
| {/* button children */} | |
| </Button> | |
| ) | |
| ); |
🤖 Prompt for AI Agents
In shesha-reactjs/src/components/fileUpload/index.tsx around line 247, the
inline style sets justifyContent to props?.style?.textAlign || 'left', but
'left' is not a valid flex value; translate common text-align values to their
flex equivalents before assigning: map 'left' or 'start' → 'flex-start', 'right'
or 'end' → 'flex-end', 'center' → 'center', 'justify' → 'space-between' (or
another desired flex justify), and default to 'flex-start'; implement a small
helper or inline conditional that reads props?.style?.textAlign, normalizes it
(toLowerCase), maps to the correct justifyContent value, and use that mapped
value in the style object.
2 participants
#3711
Summary by CodeRabbit
New Features
Style