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
WalkthroughRemoved an extra wrapper in CustomFile and added a default export; added an outer wrapper element and computed styling values to StoredFilesRendererBase, changed the disabled+no-files branch to return null, wired downloadZip into styles/model usage, and exposed a new storedFilesRendererWrapper style token. Changes
Sequence Diagram(s)sequenceDiagram
participant CustomFile
participant StoredFilesRendererBase
participant Styles
CustomFile->>StoredFilesRendererBase: render(props)
StoredFilesRendererBase->>Styles: useStyles({ containerStyles, downloadZip })
Styles-->>StoredFilesRendererBase: class names & tokens (includes storedFilesRendererWrapper)
alt disabled && noFiles
StoredFilesRendererBase->>StoredFilesRendererBase: return null
else
StoredFilesRendererBase-->>CustomFile: rendered output wrapped by storedFilesRendererWrapper
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts (1)
203-204: Significant layout change - flex direction for item containers.The ant-upload-list-item-container now uses
display: flex !important; flex-direction: row, which is a substantial layout change from the previous rendering approach. Please ensure this doesn't cause unintended visual regressions, particularly with:
- Item alignment and spacing across different layout modes (horizontal, vertical, grid)
- Thumbnail rendering and positioning
- File name and action button placement
Consider adding visual regression tests or manual testing across all layout configurations to verify the flex layout change works as intended.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
shesha-reactjs/src/components/customFile/index.tsx(1 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)
🧰 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/configuration-studio/styles.tsshesha-reactjs/src/components/storedFilesRendererBase/styles/styles.tsshesha-reactjs/src/components/sidebarContainer/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/configuration-studio/styles.tsshesha-reactjs/src/components/storedFilesRendererBase/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/sidebarContainer/styles/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/customFile/index.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 (7)
shesha-reactjs/src/components/sidebarContainer/styles/styles.ts (1)
53-53: LGTM! Scrolling enabled for open sidebars.The addition of
overflow: scrollto the open sidebar state is appropriate and will allow users to scroll through sidebar content that exceeds the visible area.shesha-reactjs/src/components/customFile/index.tsx (2)
48-67: LGTM! Wrapper removal aligns with refactoring.The removal of the intermediate wrapper div is appropriate since
StoredFilesRendererBasenow manages its own wrapper internally (viastoredFilesRendererWrapper). The props are correctly passed through without any changes to the logic.
71-71: LGTM! Default export added.Adding a default export alongside the existing named export provides flexibility for consumers without breaking existing imports.
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx (2)
287-287: LGTM! Wrapper introduced for layout management.The new outer wrapper div with
storedFilesRendererWrapperclass aligns with the styling refactoring and provides a consistent container for margin/padding management.
307-309: Null rendering is intentional pre-existing behavior, not a new change.This conditional rendering (
props.disabled && fileList.length === 0 ? null) existed in the codebase before the recent refactoring commits and is not a behavior change introduced by this PR. The component maintains a stable layout structure via the outer wrapper div (storedFilesRendererWrapper), preventing layout collapse. The logic is intentional: when disabled and empty, the upload UI is hidden while the file list (if present) remains viewable. No parent components or form validation logic depends on this component always rendering content.shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts (2)
30-39: LGTM! Wrapper styles extracted for better separation of concerns.The new
storedFilesRendererWrapperappropriately handles container-level margins and padding, separating these concerns from the main renderer styles. This aligns with the refactoring to support an outer wrapper div.
383-383: LGTM! Wrapper export added to public API.The addition of
storedFilesRendererWrapperto the exports is necessary for its usage in the component and maintains a consistent public API.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts (2)
75-75: Consider adding a comment to clarify the CSS variable override.The
--ant-margin-xsvariable is set to${gap ?? '8px'}at the root level (line 44) and in the upload list (line 186), but overridden to0pxhere within individual items. While this scoped override is likely intentional to control spacing precisely, a brief comment would help maintainers understand why the margin is zeroed at the item level.
92-92: Consider the positioning of the spread operator.Spreading
${rest}before explicitly defined properties (background, border, etc.) works due to CSS cascade, but placing it last or adding a comment would make the override intent clearer to future maintainers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts(13 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 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-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.ts
🔇 Additional comments (6)
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts (6)
30-39: LGTM: Good separation of layout concerns.The new
storedFilesRendererWrapperappropriately extracts container-level margins and padding from the main renderer styles, providing better modularity and reusability.
101-103: LGTM: Background property additions improve styling control.The explicit background properties with
!importantflags provide finer-grained control over thumbnail backgrounds and align with the codebase's styling patterns.
144-146: LGTM: Responsive drag area height.Setting the upload drag area to
height: max-contentallows it to resize dynamically based on its content, improving responsiveness.
204-205: LGTM: Flex layout improves alignment.Changing to
display: flexwithflex-direction: rowprovides better control over item alignment and is appropriate for the container layout.
345-346: LGTM: Consistent spacing for grid layout.The margin adjustments (removing top margin, setting 8px bottom margin) provide consistent spacing between grid items.
384-384: LGTM: Necessary export for wrapper consumption.The
storedFilesRendererWrapperis correctly exported, enabling it to be used by components that consume this styles hook.
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts (1)
30-39: Guard wrapper margins againstundefinedoutput
When a container style omits, say,marginTop, this template literal now emitsmargin-top: undefined;, which ends up verbatim in the generated CSS. Emotion doesn’t strip it, so we inject invalid declarations into the stylesheet. Please default these margins to0px(similar to how you handled paddings) or skip the rule when the value is missing.const storedFilesRendererWrapper = cx("stored-files-renderer-wrapper", css` - margin-top: ${marginTop}; - margin-left: ${marginLeft}; - margin-right: ${marginRight}; - margin-bottom: ${marginBottom}; + margin-top: ${marginTop ?? '0px'}; + margin-left: ${marginLeft ?? '0px'}; + margin-right: ${marginRight ?? '0px'}; + margin-bottom: ${marginBottom ?? '0px'}; padding-top: ${paddingTop ?? '2px'}; padding-left: ${paddingLeft ?? '2px'}; padding-right: ${paddingRight ?? '2px'}; padding-bottom: ${paddingBottom ?? '2px'}; `);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx(2 hunks)shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts(12 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- shesha-reactjs/src/components/storedFilesRendererBase/index.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 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-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.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/storedFilesRendererBase/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
- Add support for left and right text alignment via justify-content - Remove max-height constraint from container - Add downloadZip prop and adjust container height styles - Clean up unrelated styles - Restore text align functionality 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
czwe-01
force-pushed
the
thulasizwe/fix/4098
branch
from
3ec4c76 to
3452ad7
Compare
czwe-01
requested review from
IvanIlyichev
and removed request for
IvanIlyichev
…StoredFilesRendererBase component
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts
Outdated
Show resolved
Hide resolved
…t variable naming - Ensure consistent usage of variables for dimensions, font properties, and border radius - Clean up redundant styles and improve maintainability
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts (2)
86-98: Consider the risks of spreadingrestbefore explicit properties.The
restspread at line 87 occurs before explicit style declarations (background, border, etc.). While explicit properties should override, this pattern can be fragile ifrestcontains conflicting CSS that uses!importantor if the order matters for specificity.
282-282: Reconsider overriding the CSS variable in nested context.This line overrides the
--sha-container-widthvariable that was defined in the parent.sha-stored-files-rendererclass (line 46). Per the past review feedback from IvanIlyichev, modifying CSS variables at nested component levels can be a source of bugs and should be avoided.Consider using a different variable name for vertical-specific width (e.g.,
--sha-vertical-width) or applying the width directly without overriding the parent variable. Based on learnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts(14 hunks)
🧰 Additional context used
🧠 Learnings (7)
📚 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-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/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/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: 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/storedFilesRendererBase/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/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
⏰ 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/storedFilesRendererBase/styles/styles.ts (4)
30-39: LGTM!The new wrapper class cleanly separates outer container spacing concerns from the internal renderer styles.
294-297: LGTM - downloadZip integration is properly handled.The calc expressions correctly adjust container heights based on the
downloadZipflag, subtracting 32px for the button container when present. The pattern is consistent across all layout variants.
383-383: LGTM - New wrapper export for outer container styling.The addition of
storedFilesRendererWrapperto the exports properly exposes the new wrapper class for external consumption, aligning with the refactored styling architecture.
55-55: Unable to verify restContainerStyles due to inaccessible repository files.The repository files for
shesha-reactjs/src/components/storedFilesRendererBase/could not be located in the sandbox environment. Without access to the actual code, I cannot verify:
- How
restContainerStylesis populated and what properties it contains- Whether spreading it into the CSS block poses style conflicts or invalid CSS
- The overall safety of this pattern
Please manually verify that
restContainerStylescontains only valid CSS property objects before merging, or provide the actual file content for analysis.
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts (3)
82-87: Consider more targeted style property spreading.The
${rest}object is spread across multiple distinct selectors (lines 82, 87, 145, 225). Sincerestcontains all undestructured style properties, this could lead to unintended style application where properties meant for one element affect others.Consider:
- Creating separate style property groups for different element types (e.g.,
thumbnailStyles,uploadStyles,stubStyles)- Being more explicit about which properties apply to which selectors
- Documenting which style properties are expected to be applied globally via
restThis would make the styling more predictable and easier to debug.
127-127: Consider extracting magic number to a named constant.The calc expression multiplies
fontSizeby4, but this magic number isn't documented. While the fallback?? '14px'prevents undefined, consider:
- Extracting the multiplier (4) to a named constant with documentation explaining why this value is used
- Adding a comment explaining the calculation purpose
Example:
// Account for header, footer, padding, and button height const VERTICAL_SPACING_MULTIPLIER = 4; // Then use: calc(${isDragger ? '0px' : fontSize ?? '14px'} * ${VERTICAL_SPACING_MULTIPLIER})
209-209: Consider extracting alignment mapping to a helper function.The textAlign-to-justify-content mapping is now semantically correct (addressing the previous review concern), but the nested ternary is lengthy and appears multiple times in the codebase.
For improved readability and reusability, consider extracting to a helper:
const mapTextAlignToJustifyContent = (align?: string): string => { const mapping: Record<string, string> = { 'left': 'flex-start', 'right': 'flex-end', 'center': 'center', }; return mapping[align ?? 'center'] ?? 'center'; };Then use:
justify-content: ${mapTextAlignToJustifyContent(textAlign)};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts(14 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 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-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.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/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: 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/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
⏰ 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 (5)
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts (5)
30-39: LGTM: Wrapper implementation is clean.The new
storedFilesRendererWrapperproperly encapsulates spacing styles with sensible fallback values for padding properties.
326-332: LGTM: Grid layout flex properties are well-configured.The flex properties for the grid layout upload list are properly set to create a wrapping grid with top alignment, which aligns with the expected grid behavior.
372-383: LGTM: Public API addition is properly structured.The
storedFilesRendererWrapperis correctly added to the return object, making it available for external usage alongside the other exported styles. This aligns with the wrapper-based styling approach documented in the AI summary.
282-282: Verify that overriding--sha-container-widthdoesn't affect child components.This line overrides the
--sha-container-widthCSS variable (originally set at line 46) tomax-contentspecifically for the vertical layout. Please manually verify that no child selectors or nested components within this layout depend on the originalcontainerWidthvalue, as this override with!importantcould cause unexpected layout issues.
41-55: Unable to verify the storedFilesRendererBase component—the file could not be located in the codebase despite multiple search attempts. The file pathshesha-reactjs/src/components/storedFilesRendererBase/styles/styles.tsand search patterns for--sha-prefixed CSS variables returned no results.Related learnings from prior PRs show that component-level CSS variable usage with scoped prefixes is intentionally used in similar components (e.g., quickViewContent, hint popovers), which may indicate this approach is consistent with project patterns.
Please confirm:
- The correct file path for the storedFilesRendererBase component
- Whether the CSS variable scoping aligns with the project's established patterns (particularly regarding component-level CSS variable modification)
- Whether these
--sha-prefixed variables are intended to be component-local or shared across components
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts
Outdated
Show resolved
Hide resolved
IvanIlyichev
left a comment
IvanIlyichev
left a comment
There was a problem hiding this comment.
Hi @czwe-01. Please remove declaration of css variables and any modifications of existing ones (I'll provide detailed explanation in PM). All comments from Coderabbit must be reviewed and marked as resolved
…ted values for dimensions and font properties - Replace CSS variables with computed values for thumbnail dimensions, gap, font size, weight, and family - Improve maintainability by reducing redundancy in style definitions - Ensure consistent application of styles across different layout scenarios
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts (1)
122-122: Simplify nested calc() expression.The inner
calc()incalc(${containerMaxHeight} - calc(${isDragger ? '0px' : fontSizeValue} * 4))is redundant. CSS calc() can handle the multiplication directly without nesting.Apply this diff to simplify:
- ${containerMaxHeight ? `max-height: calc(${containerMaxHeight} - calc(${isDragger ? '0px' : fontSizeValue} * 4)) !important;` : ''} + ${containerMaxHeight ? `max-height: calc(${containerMaxHeight} - (${isDragger ? '0px' : fontSizeValue} * 4)) !important;` : ''}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts(17 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 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-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.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: 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/storedFilesRendererBase/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/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
⏰ 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 (3)
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts (3)
22-28: LGTM! Clean refactoring to computed values.The introduction of computed values with sensible defaults (e.g., '14px' for fontSize, '8px' for gap) improves type safety and eliminates potential undefined values in CSS. This approach is clearer than relying on CSS variables with fallbacks.
38-47: LGTM! Wrapper provides proper layout scoping.The new wrapper class correctly handles margin and padding with reasonable defaults, providing proper scoping for the component's layout as described in the PR changes.
366-378: LGTM! Return statement properly exposes all classes.The return statement correctly includes the new
storedFilesRendererWrapperclass along with all other necessary class names for external usage.
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts
Outdated
Show resolved
Hide resolved
…ust dimensions based on downloadZip button visibility - Introduce computed value for downloadZip button height to streamline style calculations
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts (2)
121-126: Extract hardcoded dimension values to constants.Multiple
32pxvalues are hardcoded in calc expressions throughout this block. WhiledownloadZipBtnHeightwas introduced (line 28) to address similar concerns elsewhere, these values remain hardcoded. If they represent UI element dimensions (padding, borders, button heights, etc.), extract them to named constants for easier maintenance.Apply this pattern:
+ const LIST_TEXT_OFFSET = '32px'; + const thumbnailWidth = layout ? (width ?? '54px') : '100%';Then replace the hardcoded values:
- ${containerHeight ? `height: calc(${containerHeight} - 32px) !important;` : ''} + ${containerHeight ? `height: calc(${containerHeight} - ${LIST_TEXT_OFFSET}) !important;` : ''}Apply similar changes to lines 123-126.
122-122: Consider documenting the magic number multiplier.The calc expression uses
* 4to multiply the font size, likely to account for text height and spacing. Consider extracting this to a named constant (e.g.,TEXT_HEIGHT_MULTIPLIER = 4) or adding a comment explaining the calculation.Example:
+ const TEXT_HEIGHT_MULTIPLIER = 4; // Accounts for line height and padding + const thumbnailWidth = layout ? (width ?? '54px') : '100%';Then update line 122:
- ${containerMaxHeight ? `max-height: calc(${containerMaxHeight} - calc(${isDragger ? '0px' : fontSizeValue} * 4)) !important;` : ''} + ${containerMaxHeight ? `max-height: calc(${containerMaxHeight} - calc(${isDragger ? '0px' : fontSizeValue} * ${TEXT_HEIGHT_MULTIPLIER})) !important;` : ''}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts(17 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 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-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.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-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/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: 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/storedFilesRendererBase/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 (5)
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts (5)
22-28: Good refactor introducing computed styling values.The extraction of these computed values with sensible defaults improves maintainability and readability throughout the style definitions.
38-47: Wrapper pattern properly scopes container styles.The introduction of
storedFilesRendererWrappercorrectly isolates container margin/padding concerns, addressing previous feedback about avoiding component-level CSS variable modifications.
288-291: Correct usage of extracted constant.The
downloadZipBtnHeightconstant is properly used here, making the relationship to the download ZIP feature clear and maintainable.
327-329: Correct usage of extracted constant.The
downloadZipBtnHeightconstant is properly used in the grid layout calculations.
376-376: LGTM - wrapper properly exposed.The new
storedFilesRendererWrapperis correctly added to the return object for use in the component.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts (1)
120-126: Extract hardcoded offset to a constant.The
32pxoffset is repeated five times in this block. While this appears to be a padding/border offset (different from the download button height since it affects both width and height), extracting it to a named constant would improve maintainability.Apply this diff to extract the constant:
const fontFamilyValue = fontFamily ?? 'Segoe UI'; const downloadZipBtnHeight = model.downloadZip ? '32px' : '0px'; + const listTextOffset = '32px';Then update the usages:
.ant-upload-list-text { - ${containerHeight ? `height: calc(${containerHeight} - 32px) !important;` : ''} + ${containerHeight ? `height: calc(${containerHeight} - ${listTextOffset}) !important;` : ''} ${containerMaxHeight ? `max-height: calc(${containerMaxHeight} - calc(${isDragger ? '0px' : fontSizeValue} * 4)) !important;` : ''} - ${containerMinHeight ? `min-height: calc(${containerMinHeight} - 32px) !important;` : ''} + ${containerMinHeight ? `min-height: calc(${containerMinHeight} - ${listTextOffset}) !important;` : ''} - ${containerWidth ? `width: calc(${containerWidth} - 32px) !important;` : ''} + ${containerWidth ? `width: calc(${containerWidth} - ${listTextOffset}) !important;` : ''} - ${containerMaxWidth ? `max-width: calc(${containerMaxWidth} - 32px) !important;` : ''} + ${containerMaxWidth ? `max-width: calc(${containerMaxWidth} - ${listTextOffset}) !important;` : ''} - ${containerMinWidth ? `min-width: calc(${containerMinWidth} - 32px) !important;` : ''} + ${containerMinWidth ? `min-width: calc(${containerMinWidth} - ${listTextOffset}) !important;` : ''} }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts(17 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 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-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.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/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: 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/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 (4)
shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts (4)
22-28: LGTM! Well-structured computed values.The computed styling values provide good defensive defaults and make the template literals more readable. The
downloadZipBtnHeightconstant properly addresses the previous review feedback.
38-47: LGTM! Properly scoped wrapper styling.The new
storedFilesRendererWrapperprovides proper scoping for margin and padding styles fromcontainerStyles, with sensible defaults for padding.
49-56: LGTM! Proper scoping with conditional inline CSS.The refactoring to use conditional inline CSS rules instead of CSS variable modifications properly addresses the architectural concern raised in previous reviews about avoiding CSS variable modification at component level.
67-67: LGTM! Consistent use of computed typography and dimension values.The computed values (
fontSizeValue,fontWeightValue,fontFamilyValue,thumbnailWidth,thumbnailHeight,gapValue) are used consistently throughout the stylesheet, and all have safe defaults to prevent undefined values in the generated CSS.Also applies to: 96-97, 112-115, 163-166, 175-175
2 participants
#4098
Summary by CodeRabbit
Style
Bug Fixes
New Features
Chores