Merged
Conversation
…and update styles for better layout
…t layout handling
Contributor
WalkthroughReplaces inline filename rendering with a new FileNameDisplay component (tooltip showing human-readable size) used across list and thumbnail views, removes upload trigger onClick in non-dragger mode, removes Changes
Sequence Diagram(s)(Skipped — UI rendering/styling changes; no new multi-actor sequential flow meeting diagram criteria.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 5
🤖 Fix all issues with AI agents
In `@shesha-reactjs/src/components/storedFilesRendererBase/index.tsx`:
- Around line 444-449: The wrapper div and FileNameDisplay both receive the same
styles.fileName, causing nested elements with identical classes; remove the
duplicate by dropping the className prop from the FileNameDisplay call (leave
styles.fileName on the outer div) so FileNameDisplay does not receive className,
or alternatively remove the class from the outer div if you prefer the inner to
own styling—update the JSX around Popover/iconRender/FileNameDisplay accordingly
to only apply styles.fileName once.
In `@shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts`:
- Around line 256-277: The CSS block for .ant-upload-list-text has a misplaced
child selector (> .downloaded-icon) and an extra closing brace; move the >
.downloaded-icon rule either inside the .ant-upload-list-text block before its
closing brace or convert it into a separate top-level selector, and remove the
unmatched closing brace so braces balance; update the styles around
.ant-upload-list-text and > .downloaded-icon accordingly to maintain intended
scoping and remove the syntax error.
- Around line 106-121: The CSS-in-JS block for .ant-typography accesses
downloadedFileStyles.color without optional chaining which can throw if
downloadedFileStyles is undefined; update the usage in the .ant-typography rule
to use downloadedFileStyles?.color (consistent with
downloadedFileStyles?.fontSize, ?.fontWeight, ?.fontFamily, ?.textAlign) so all
references to downloadedFileStyles are null-safe and won’t cause runtime errors
in styles.ts.
In `@shesha-reactjs/src/components/storedFilesRendererBase/utils.tsx`:
- Around line 177-183: The formatFileSize function currently checks for null
only, which lets undefined fall through and produce NaN; update the initial
guard in formatFileSize to treat both null and undefined (e.g., replace the
`bytes === null` check with a nullish/loose equality check like `bytes == null`
or an explicit `bytes === undefined || bytes === null`) so the function returns
an empty string when bytes is missing and avoids running the arithmetic that
yields "NaN GB".
- Around line 176-183: Replace the custom formatting in formatFileSize with the
existing filesize library to ensure consistent output; update the
formatFileSize(bytes?: number) implementation to return an empty string for
null/undefined and otherwise call filesize(bytes) (or filesize(bytes, { base: 2
} if you need KB/MB as powers of 1024 to match current behavior), preserving the
function signature so callers like FileVersionsButton keep working and types
remain correct.
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx
Outdated
Show resolved
Hide resolved
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@shesha-reactjs/src/components/storedFilesRendererBase/index.tsx`:
- Around line 482-485: The thumbnail filename is rendered even when isDragger is
true (dragger mode shouldn't render thumbnail UI); update the condition that
currently checks listType === 'thumbnail' to also require !isDragger (e.g.,
change to !isDragger && listType === 'thumbnail') or normalize listType earlier
so FileNameDisplay (used here along with isDownloaded and styles.fileName) is
only rendered for non-dragger thumbnail mode; ensure this prevents
duplicate/awkward UI in dragger mode.
♻️ Duplicate comments (2)
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx (1)
444-449: Remove duplicatestyles.fileNameclass on nested wrappers.
styles.fileNameis applied to both the outer<div>andFileNameDisplay, which already wraps its own container. This can double-apply ellipsis/layout styles. Keep it on just one element.♻️ Suggested change
- <Popover content={actions} trigger="hover" placement="top" classNames={{ root: styles.actionsPopover }}> - <div className={styles.fileName}> - {iconRender(file)} - <FileNameDisplay file={file} className={styles.fileName} /> - </div> - </Popover> + <Popover content={actions} trigger="hover" placement="top" classNames={{ root: styles.actionsPopover }}> + <div> + {iconRender(file)} + <FileNameDisplay file={file} className={styles.fileName} /> + </div> + </Popover>shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts (1)
107-123: Avoid unsafe access ofdownloadedFileStyles.color.Line 111 dereferences
downloadedFileStyles.colorwithout optional chaining, which can throw if the prop is undefined. UsedownloadedFileStyles?.colorlike the other fields.🐛 Suggested fix
- color: ${downloadedFileStyles.color ?? color} !important; + color: ${downloadedFileStyles?.color ?? color} !important;
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx
Outdated
Show resolved
Hide resolved
…e name visibility
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts`:
- Line 116: The CSS template currently concatenates the conditional
`justify-content` with `margin` on one line; refactor the string in styles.ts so
the `justify-content` expression (using `downloadedFileStyles?.textAlign` and
its ternary branches) is its own CSS declaration ending with a semicolon, and
place `margin: 2px 0px;` on the next line as a separate declaration to match the
readability of the `fileName` class; update the template literal where
`downloadedFileStyles?.textAlign` is referenced to produce `justify-content:
center|flex-end|flex-start;` and then add `margin: 2px 0px;` after it.
♻️ Duplicate comments (1)
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx (1)
445-450: Duplicatestyles.fileNameclass application.The
styles.fileNameclass is applied to both the wrapper<div>(line 446) and passed toFileNameDisplay(line 448). SinceFileNameDisplayapplies the className to its internal wrapper, this creates nested elements with the same class, potentially causing unintended style cascading.♻️ Consider removing the duplicate className from the outer div
<Popover content={actions} trigger="hover" placement="top" classNames={{ root: styles.actionsPopover }}> - <div className={styles.fileName}> + <div> {iconRender(file)} <FileNameDisplay file={file} className={styles.fileName} /> </div> </Popover>
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts`:
- Around line 308-318: The CSS-in-JS injection uses the short-circuit pattern
that can produce the literal "false" when hasFiles is falsy; update the
expression inside the .${prefixCls}-upload-drag block to use a ternary so it
emits the rule only when hasFiles is true (e.g., replace the ${hasFiles &&
'border: unset !important;'} usage with a ternary that returns the string when
hasFiles is truthy and an empty string otherwise), targeting the styles.ts code
around the .${prefixCls}-upload-drag / ${prefixCls}-upload-btn definitions so
the conditional border rule no longer injects "false".
- Around line 259-277: The template literal in the .ant-upload-list-text block
injects the literal "false" when hasFiles is true; replace the short-circuit
expression `${!hasFiles && 'display: none;'}` with a ternary that outputs either
the CSS or an empty string (e.g. `${!hasFiles ? 'display: none;' : ''}`) so no
"false" text is produced; update the CSS inside the .ant-upload-list-text rule
where hasFiles is referenced.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts`:
- Around line 279-286: The CSS template is using redundant nested template
literals for width/height; replace `${`width: ${layout ? thumbnailWidth :
'100%'} !important;`}` and similar lines with direct template interpolations
like `width: ${layout ? thumbnailWidth : '100%'} !important;` and do the same
for `height` and the `.ant-upload` block so the expressions for `layout`,
`thumbnailWidth`, and `thumbnailHeight` are interpolated directly without the
extra `${`...`}` wrapper.
- Around line 42-72: The styles duplicate typography rules between the parent
const fileName (class "item-file-name") and its nested ".ant-typography"; remove
the redundant typography properties from the parent and keep them on
".ant-typography" (font-size, font-weight, font-family, text-align,
justify-content, color) so the nested element controls text rendering, while
preserving parent-only layout rules (display, gap, white-space, overflow,
text-overflow, min-width) and the conditional visibility using
model.hideFileName in ".ant-typography".
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts
Outdated
Show resolved
Hide resolved
…t and update styles for improved file name visibility
James-Baloyi
approved these changes
Jan 27, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@shesha-reactjs/src/components/storedFilesRendererBase/utils.tsx`:
- Around line 186-216: The inline props type for the FileNameDisplay component
should be extracted into a named interface for clarity and reusability: create
and export a new interface (e.g., FileNameDisplayProps) that defines file:
UploadFile; className?: string; popoverContent?: React.ReactNode;
popoverClassName?: string, then update the component declaration to use
FC<FileNameDisplayProps> and replace the inline destructured type with the same
props name in the parameter list; ensure any existing imports/types (UploadFile,
React) remain valid and adjust any other references to this component if
necessary.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@shesha-reactjs/src/components/storedFilesRendererBase/index.tsx`:
- Around line 447-452: The Popover used inside FileNameDisplay is only passing
popoverClassName to the Popover root, so the popover body doesn't receive the
same styling; update the Popover invocation (the one rendering textElement
inside FileNameDisplay / storedFilesRendererBase's component) to pass classNames
with both root and body keys (i.e., classNames={{ root: popoverClassName, body:
popoverClassName }}) so the popover body uses the same styles as the root.
James-Baloyi
approved these changes
Jan 27, 2026
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.
#4098
Summary by CodeRabbit
New Features
Style
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.