-
Notifications
You must be signed in to change notification settings - Fork 123
File list: Fix icon size in desinger #4429
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: releases/0.43
Are you sure you want to change the base?
Conversation
WalkthroughAdds a new FileNameDisplay component and integrates it across storedFilesRendererBase render paths, replacing inline filename text with an ellipsized Typography component (title shows formatted size). Adjusts thumbnail/text render markup, wraps list items in Popover for actions, shifts filename styling into nested Typography rules, and tweaks upload icon sizing. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
…d file name display - Added a helper function to format file sizes for better readability. - Introduced a new FileNameDisplay component to render file names with ellipsis and tooltips for size information. - Updated various instances in the component to utilize the new FileNameDisplay for consistent file name rendering. - Adjusted styles to ensure proper display of file names using Ant Design Typography.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@shesha-reactjs/src/components/storedFilesRendererBase/index.tsx`:
- Around line 96-103: The formatFileSize helper incorrectly treats 0 as falsy so
formatFileSize(0) returns an empty string; update the initial guard in
formatFileSize to only bail on null/undefined (e.g., check bytes == null or
bytes === undefined || bytes === null) so a 0 value is processed and returns "0
B" as expected, leaving the rest of the sizing logic in formatFileSize
unchanged.
- Around line 365-369: The render currently outputs raw bytes via `{file.size}`
inside the <p className="ant-upload-list-item-name"> which shows unformatted
numbers; replace that with the intended display component/formatting (use the
existing FileNameDisplay component or a size formatter). Locate the block that
renders Image and the <p> (symbols: Image, file, imageUrls[uid], className
"ant-upload-list-item-name") and change the <p> content to render
<FileNameDisplay file={file} /> (or call the repository's formatBytes/fileSize
utility with file.size) so the file name/size is shown in a human-friendly
format instead of the raw number.
In `@shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts`:
- Line 55: The CSS declaration in the styled template that sets font-size is
concatenating two rules on one line ("font-size: ${addPx(downloadedFileFontSize)
?? fontSize} !important;margin: 2px 0px;"); split these into two separate
declarations and lines for readability and consistency by placing "font-size:
${addPx(downloadedFileFontSize) ?? fontSize} !important;" and "margin: 2px 0px;"
on their own lines within the same template (referencing the
downloadedFileFontSize and fontSize expressions in styles.ts).
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
shesha-reactjs/src/components/storedFilesRendererBase/index.tsxshesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts
🧰 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/storedFilesRendererBase/index.tsxshesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts
🧠 Learnings (3)
📚 Learning: 2026-01-14T18:34:34.784Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4408
File: shesha-reactjs/src/components/formDesigner/formComponent.tsx:115-115
Timestamp: 2026-01-14T18:34:34.784Z
Learning: In shesha-reactjs/src/components/formDesigner/formComponent.tsx, the memoized validation logic includes actualModel in the dependency array alongside toolboxComponent, getValidation, and errorCount. Since actualModel is mutated after creation from useActualContextData, including it in the dependency array should not be flagged as a memoization issue. Treat this pattern as acceptable for this file; do not raise memoization-warn checks here.
Applied to files:
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx
📚 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/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/storedFilesRendererBase/styles/styles.ts
🔇 Additional comments (6)
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx (5)
106-121: LGTM!The
FileNameDisplaycomponent is well-structured with proper TypeScript typing. It correctly usesTypography.Textwith ellipsis and computes a meaningful title including file size.
654-660: LGTM!The
PictureOutlinedicon now correctly uses the dynamiciconSizeprop for itsfontSize, which addresses the PR objective of fixing icon size in the designer.
544-559: LGTM!The Popover structure correctly wraps both the icon and
FileNameDisplay, ensuring the entire file item triggers the hover actions. TheonOpenChangecallback properly sets the current file context.
601-605: LGTM!Consistent use of
FileNameDisplayfor the thumbnail list type, with the appropriateclassNamefor styling.
662-665: LGTM!The placeholder/stub render path correctly uses
FileNameDisplaywith theplaceholderFile, maintaining consistency across all render paths.shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts (1)
25-36: Consider extracting shared typography styles to reduce duplication.The
.ant-typographystyles infileName(lines 25-36) and insidedownloadedFile > .item-file-name(lines 52-62) share most properties. Consider extracting common styles into a reusable mixin or base object, then applying only the overrides (color, font-size) in thedownloadedFilecontext.Also applies to: 51-64
⛔ Skipped due to 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: 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.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
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
…style adjustments - Updated file size formatting logic to handle null values correctly. - Changed displayed property from file size to file name for better clarity. - Enhanced styles to support text alignment and responsive design for file names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
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/storedFilesRendererBase/styles/styles.ts (1)
170-194: Fix invalidwidth; 100%declaration.The semicolon typo prevents the width rule from applying, which can break alignment/ellipsis in the text list layout.
🛠️ Proposed fix
- width; 100%; + width: 100%;
🤖 Fix all issues with AI agents
In `@shesha-reactjs/src/components/storedFilesRendererBase/index.tsx`:
- Around line 365-368: Replace the incorrect property access to file.fileName
with the proper display component: use the existing FileNameDisplay component
(instead of the literal <p
className="ant-upload-list-item-name">{file.fileName}</p>) so it reads file.name
and shows size; update the render block that contains Image and the p tag to
render <FileNameDisplay file={file} /> (or the project equivalent) to ensure
newly added UploadFile entries display correctly.
♻️ Duplicate comments (1)
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx (1)
96-103: GuardformatFileSizeagainstundefinedto avoidNaNoutput.
bytesis optional, andundefinedcurrently falls through to the GB branch, producingNaN GB.🛠️ Proposed fix
-const formatFileSize = (bytes?: number): string => { - if (bytes === null) return ''; +const formatFileSize = (bytes?: number): string => { + if (bytes == null) return ''; if (bytes < 1024) return `${bytes} B`; if (bytes < 1024 * 1024) return `${(bytes / 1024).toFixed(2)} KB`; if (bytes < 1024 * 1024 * 1024) return `${(bytes / (1024 * 1024)).toFixed(2)} MB`; return `${(bytes / (1024 * 1024 * 1024)).toFixed(2)} GB`; };
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
shesha-reactjs/src/components/storedFilesRendererBase/index.tsxshesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts
🧰 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/storedFilesRendererBase/index.tsxshesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts
🧠 Learnings (7)
📚 Learning: 2026-01-14T18:34:34.784Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4408
File: shesha-reactjs/src/components/formDesigner/formComponent.tsx:115-115
Timestamp: 2026-01-14T18:34:34.784Z
Learning: In shesha-reactjs/src/components/formDesigner/formComponent.tsx, the memoized validation logic includes actualModel in the dependency array alongside toolboxComponent, getValidation, and errorCount. Since actualModel is mutated after creation from useActualContextData, including it in the dependency array should not be flagged as a memoization issue. Treat this pattern as acceptable for this file; do not raise memoization-warn checks here.
Applied to files:
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx
📚 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/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/storedFilesRendererBase/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/storedFilesRendererBase/styles/styles.ts
📚 Learning: 2026-01-15T13:08:16.063Z
Learnt from: czwe-01
Repo: shesha-io/shesha-framework PR: 4421
File: shesha-reactjs/src/designer-components/attachmentsEditor/settings.ts:496-500
Timestamp: 2026-01-15T13:08:16.063Z
Learning: In shesha-reactjs designer components, the `iconSize` property accepts both `string | number` types, so it should use `inputType: 'textField'` rather than `numberField` to allow values like "16px", "1em", or plain numbers.
Applied to files:
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts
📚 Learning: 2025-12-11T11:57:55.470Z
Learnt from: czwe-01
Repo: shesha-io/shesha-framework PR: 4349
File: shesha-reactjs/src/designer-components/columns/styles.ts:3-14
Timestamp: 2025-12-11T11:57:55.470Z
Learning: In the shesha-framework, when using createStyles to define styles hooks (e.g., export const useStyles = createStyles(...)), the standard pattern is to return an object with named properties (e.g., { columns }) from the createStyles callback, and then consume it in components using const { styles } = useStyles() followed by accessing the properties via styles.propertyName (e.g., styles.columns). The createStyles utility handles the wrapping automatically.
Applied to files:
shesha-reactjs/src/components/storedFilesRendererBase/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/storedFilesRendererBase/styles/styles.ts
🔇 Additional comments (2)
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts (1)
12-69: Typography-driven filename styling looks consistent.The shift to nested
.ant-typographywith flex/ellipsis aligns well with the new display logic.shesha-reactjs/src/components/storedFilesRendererBase/index.tsx (1)
94-95: Nice consolidation of filename display + dynamic icon sizing.Using
FileNameDisplayacross text/thumbnail/stub paths and wiringiconSizeintoPictureOutlinedkeeps UI output consistent.Also applies to: 105-121, 544-559, 603-604, 655-665
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx
Outdated
Show resolved
Hide resolved
… for improved file name rendering - Replaced direct file name display with FileNameDisplay component for better styling and functionality. - Enhanced overall clarity and maintainability of the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
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/storedFilesRendererBase/index.tsx (1)
362-368: Avoid duplicate filename rendering for image thumbnails.For
listType === 'thumbnail'and image files,FileNameDisplayis rendered insideiconRender(lines 259-263) and again in the thumbnail rendering (lines 362-368 and 601-604), which results in duplicate filenames. RemoveFileNameDisplayfrom the thumbnail rendering since it's already provided byiconRender.🛠️ One possible fix (keep iconRender only)
At lines 362-368:
return ( <Space size="small" direction="vertical"> <Image src={imageUrls[uid]} alt={file.name} preview={false} /> - <FileNameDisplay file={file} className={styles.fileName} /> </Space> );At lines 601-604:
return ( <Space size="small" direction="vertical"> <Image src={imageUrls[uid]} alt={file.name} preview={false} /> - <FileNameDisplay file={file} className={styles.fileName} /> </Space> );
♻️ Duplicate comments (1)
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx (1)
96-102: Handleundefinedsize to avoidNaNdisplay.
bytesis optional; when it’sundefined, the current guard falls through and produces"NaN GB"in the title. Guard fornull/undefinedbefore comparisons.🐛 Proposed fix
const formatFileSize = (bytes?: number): string => { - if (bytes === null) return ''; + if (bytes == null) return ''; if (bytes < 1024) return `${bytes} B`; if (bytes < 1024 * 1024) return `${(bytes / 1024).toFixed(2)} KB`; if (bytes < 1024 * 1024 * 1024) return `${(bytes / (1024 * 1024)).toFixed(2)} MB`; return `${(bytes / (1024 * 1024 * 1024)).toFixed(2)} GB`; };
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx
🧰 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/storedFilesRendererBase/index.tsx
🧠 Learnings (2)
📚 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/storedFilesRendererBase/index.tsx
📚 Learning: 2026-01-14T18:34:34.784Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4408
File: shesha-reactjs/src/components/formDesigner/formComponent.tsx:115-115
Timestamp: 2026-01-14T18:34:34.784Z
Learning: In shesha-reactjs/src/components/formDesigner/formComponent.tsx, the memoized validation logic includes actualModel in the dependency array alongside toolboxComponent, getValidation, and errorCount. Since actualModel is mutated after creation from useActualContextData, including it in the dependency array should not be flagged as a memoization issue. Treat this pattern as acceptable for this file; do not raise memoization-warn checks here.
Applied to files:
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts`:
- Around line 171-189: There's a CSS syntax error in the .ant-upload-list-text
block where the .file-name-wrapper rule uses "width; 100%;" instead of "width:
100%;", causing the width to be ignored; update the .file-name-wrapper
declaration inside the .ant-upload-list-text > .ant-upload-list-item-container >
div selector to use a colon (width: 100%) so .item-file-name and responsive
alignment (textAlign ternary) behave correctly.
… file name wrapper
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts`:
- Around line 56-70: The width interpolation in the .item-file-name
.ant-typography CSS may produce invalid CSS when width is numeric/undefined;
change the width expression to use the addPx helper with a stable fallback (e.g.
width: ${addPx(width) ?? '100%'};) so it always emits a valid CSS value and
preserves text-overflow/ellipsis behavior; update the .item-file-name
.ant-typography block where width is currently interpolated and keep other
properties (display, color, text-align, font-size using
addPx(downloadedFileFontSize) etc.) unchanged.
- Moved file size formatting and file name display logic to utils.tsx for better organization. - Removed unused Typography import from index.tsx. - Updated StoredFilesRendererBase to utilize the new FileNameDisplay component for consistent file name rendering. - Enhanced code clarity and maintainability by consolidating related functionalities.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@shesha-reactjs/src/components/storedFilesRendererBase/utils.tsx`:
- Around line 178-184: The formatFileSize function fails when bytes is undefined
and produces "NaN GB"; update the guard to treat both null and undefined as
empty (e.g., check bytes == null or typeof bytes !== 'number') so the function
returns '' for missing values before any numeric comparisons; keep the rest of
the logic in formatFileSize unchanged so valid numeric sizes still format to
B/KB/MB/GB.
Summary by CodeRabbit
New Features
Bug Fixes
Style
✏️ Tip: You can customize this high-level summary in your review settings.