Closed
Conversation
- Removed unnecessary rendering of upload content when disabled and file list is empty. - Fixed double semicolon in styles and added missing max-height property for better layout control. - Adjusted margins for improved spacing in upload list items.
… layout - Added overflow scroll to the open sidebar for better content visibility. - Changed display property to flex and adjusted alignment in storedFilesRendererBase for consistent item layout. - Set height for csContent to ensure proper spacing in the configuration studio.
…yles for improved layout
Contributor
WalkthroughThis PR restructures the stored files renderer by moving the wrapper div from CustomFile to StoredFilesRendererBase, refactors the associated styles to delegate layout concerns to the new wrapper, adds overflow scrolling to the left sidebar open state, and constrains the configuration studio content height to viewport height minus 55 pixels. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–60 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)
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
🧹 Nitpick comments (1)
shesha-reactjs/src/components/sidebarContainer/styles/styles.ts (1)
53-53: Consider usingoverflow-y: autoinstead ofoverflow: scroll.The
overflow: scrollshorthand always displays both horizontal and vertical scrollbars, even when content doesn't overflow. This may create a cluttered UI. Consider usingoverflow-y: autoinstead, which:
- Only shows the vertical scrollbar when content overflows
- Prevents unwanted horizontal scrollbars (since the sidebar has a fixed 550px width)
- Aligns with the existing overflow pattern used for
.${sidebarBody}at line 101Apply this diff:
- overflow: scroll; + overflow-y: auto;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
shesha-reactjs/src/components/customFile/index.tsx(0 hunks)shesha-reactjs/src/components/sidebarContainer/styles/styles.ts(1 hunks)shesha-reactjs/src/components/storedFilesRendererBase/index.tsx(1 hunks)shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts(12 hunks)shesha-reactjs/src/configuration-studio/styles.ts(1 hunks)
💤 Files with no reviewable changes (1)
- shesha-reactjs/src/components/customFile/index.tsx
🧰 Additional context used
🧠 Learnings (4)
📚 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/storedFilesRendererBase/index.tsxshesha-reactjs/src/components/storedFilesRendererBase/styles/styles.tsshesha-reactjs/src/components/sidebarContainer/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/storedFilesRendererBase/index.tsxshesha-reactjs/src/components/storedFilesRendererBase/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/storedFilesRendererBase/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/sidebarContainer/styles/styles.ts
🧬 Code graph analysis (1)
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx (2)
shesha-reactjs/src/designer-components/attachmentsEditor/attachmentsEditor.tsx (1)
listType(27-27)shesha-reactjs/src/utils/form.ts (1)
hasFiles(89-98)
⏰ 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/configuration-studio/styles.ts (1)
55-55: Verify the 5px discrepancy in height calculation.The
csContentheight uses a literal55pxvalue, whilecsTreeAreaandcsWorkArea(lines 58, 88) both use theheaderHeightvariable (60px). This creates a 5px mismatch where the content area is 5px taller than the space allocated for its child areas.If this is intentional to account for borders, margins, or other UI elements, consider adding a comment explaining the offset. Otherwise, using the
headerHeightvariable would improve consistency.Apply this diff if the height should align with the header:
- height: calc(100vh - 55px); + height: calc(100vh - ${headerHeight}px);
Comment on lines
+287
to
+318
| <div className={styles.storedFilesRendererWrapper}> | ||
| <div className={`${styles.shaStoredFilesRenderer} ${layout === 'horizontal' && listTypeAndLayout !== 'text' ? styles.shaStoredFilesRendererHorizontal | ||
| : layout === 'vertical' && listTypeAndLayout !== 'text' ? styles.shaStoredFilesRendererVertical | ||
| : layout === 'grid' && listTypeAndLayout !== 'text' ? styles.shaStoredFilesRendererGrid : ''}`} | ||
| > | ||
| {isStub | ||
| ? (isDragger | ||
| ? <Dragger disabled ><DraggerStub styles={styles} /></Dragger> | ||
| : ( | ||
| <div | ||
| className={listType === 'thumbnail' ? 'ant-upload-list-item-thumbnail ant-upload-list-item thumbnail-stub' : ''} | ||
| > | ||
| {renderUploadContent()} | ||
| {listType !== 'text' && !rest.hideFileName && ( | ||
| <span className="ant-upload-list-item-name ant-upload-list-item-name-stub"> | ||
| file name | ||
| </span> | ||
| )} | ||
| </div> | ||
| )) | ||
| : (props.disabled && fileList.length === 0 | ||
| ? null | ||
| : props.disabled | ||
| ? <Upload {...props} style={model?.allStyles?.fullStyle} listType={listTypeAndLayout} /> | ||
| : isDragger | ||
| ? ( | ||
| <Dragger {...props}> | ||
| <DraggerStub styles={styles} /> | ||
| </Dragger> | ||
| ) | ||
| : <Upload {...props} listType={listTypeAndLayout}>{renderUploadContent()}</Upload>)} | ||
| {previewImage && ( |
Contributor
There was a problem hiding this comment.
Do not hide the entire control when disabled and empty
When props.disabled is true and fileList.length === 0, this now renders null, so the stored-files field vanishes entirely in read-only forms. Previously the disabled <Upload> still rendered a placeholder, keeping the field visible in the layout. With this change users lose any indication that the attachment slot exists, labels/descriptions become orphaned, and form sections can collapse unexpectedly. Please keep rendering a disabled list/placeholder (even if empty) so the control remains visible, or provide an explicit “No files” stub instead.
🤖 Prompt for AI Agents
In shesha-reactjs/src/components/storedFilesRendererBase/index.tsx around lines
287 to 318, the code returns null when props.disabled && fileList.length === 0
which hides the entire control in read-only forms; instead, always render a
visible disabled placeholder: change the branch that currently returns null so
it renders the disabled Upload component (or an explicit "No files" stub) with
the same props/listType/listTypeAndLayout and any fullStyle so the control
remains in the layout even when empty and disabled.
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
1 participant
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.
Summary by CodeRabbit
Release Notes
Bug Fixes
Style