Skip to content

Conversation

@teboho
Copy link
Contributor

@teboho teboho commented Jan 13, 2026

Summary by CodeRabbit

  • New Features
    • Added Advanced Statistic component for displaying metrics with customizable icons
    • Supports configurable titles (top/bottom), prefix/suffix text, and value precision formatting
    • Includes comprehensive styling, typography, and alignment customization options
    • Fully integrated into the component designer with visual configuration support

✏️ Tip: You can customize this high-level summary in your review settings.

teboho and others added 2 commits January 13, 2026 15:24
- New advancedStatistic component with comprehensive styling options
- Support for side icons (left/right), prefix/suffix with icons and text
- Single configurable title with position control (top/bottom)
- Responsive design support across desktop/tablet/mobile
- Independent styling for all elements (value, title, icons, container)
- Precision control for numeric values
- Custom event handlers and click support
- Baseline alignment for proper text/icon vertical alignment

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 13, 2026

Walkthrough

Introduces a new Advanced Statistic component for Shesha React, including a functional component with configurable value, icons, titles, and prefix/suffix elements. Comprises styling module, TypeScript interfaces, designer integration with settings form generator, utility functions, documentation, and toolbox registration for public availability.

Changes

Cohort / File(s) Summary
Component Implementation
shesha-reactjs/src/components/advancedStatistic/index.tsx, shesha-reactjs/src/components/advancedStatistic/interfaces.ts, shesha-reactjs/src/components/advancedStatistic/styles/styles.ts
New functional React component with prop interfaces (IAdvancedStatisticProps) supporting value, precision, icons, titles, prefix/suffix, fonts, and container styling. Includes CSS-in-JS styling module (useStyles hook) defining layout, spacing, typography, and component-specific classes.
Designer Integration
shesha-reactjs/src/designer-components/advancedStatistic/index.tsx, shesha-reactjs/src/designer-components/advancedStatistic/settingsForm.ts, shesha-reactjs/src/designer-components/advancedStatistic/utils.ts
Toolbox component with factory rendering, design-time editing support, dynamic styling via computed style/font helpers, memoized style objects, event handling (onClick), configuration validation, and property migration. Settings form with multi-tab UI (Common, Events, Appearance, Security tabs), nested collapsible panels, and extensive input widgets. Default styles utility interface and function.
Documentation
shesha-reactjs/src/designer-components/advancedStatistic/README.md
Feature overview, layout options, configuration properties (Value, Titles, Prefix/Suffix, Icons), styling guidance, event documentation, usage examples, layout diagrams, and versioning notes.
Toolbox Registration
shesha-reactjs/src/providers/form/defaults/toolboxComponents.ts
Registration of AdvancedStatisticComponent in Data display toolbox components list, positioned between Statistic and Text.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested reviewers

  • James-Baloyi
  • czwe-01

Poem

🐰 A new statistic hops into view,
Advanced displays in every hue,
With icons dancing left and right,
And values formatted just so tight,
Our designer now shows stats in style—
This component brings a rabbity smile! 📊✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title check ⚠️ Warning The title 'Teboho/e/3785' appears to be a branch name or issue identifier rather than a descriptive summary of the changes, which include adding a new AdvancedStatistic component with full styling and configuration support. Replace the title with a clear, descriptive summary of the main change, such as 'Add AdvancedStatistic component with styling and designer integration' or similar.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🤖 Fix all issues with AI agents
In @shesha-reactjs/src/components/advancedStatistic/index.tsx:
- Around line 121-126: The JSX return uses
classNames(styles.advancedStatisticContainer) unnecessarily; replace the
classNames call with the plain class reference by setting className to
styles.advancedStatisticContainer (remove the classNames import/use) in the
component that renders this div so the element uses
className={styles.advancedStatisticContainer} and delete the unused classNames
reference/import if no other usage remains.

In @shesha-reactjs/src/components/advancedStatistic/styles/styles.ts:
- Around line 3-14: The container currently uses a hardcoded background
("background: white") which breaks dark mode; update the useStyles/createStyles
block (specifically the advancedStatisticContainer definition inside useStyles)
to use the provided theme token instead (e.g., token.colorBgContainer or another
appropriate token like token.colorBgLayout) so the background adapts to
theme/dark mode, and remove the hardcoded white value; ensure you reference the
token param already available in createStyles and pick a token consistent with
other components.
- Around line 40-52: The title styles titleTop and titleBottom use a hardcoded
color rgba(0,0,0,0.65) which won't adapt to dark themes; update both definitions
to use the theme token (token.colorTextSecondary) instead and ensure the token
is obtained via the theme hook or context (e.g., useToken or theme tokens) in
the styles module so the color resolves from the current theme.

In @shesha-reactjs/src/designer-components/advancedStatistic/index.tsx:
- Around line 71-76: The component AdvancedStatisticComponent currently uses
BarChartOutlined as its icon which is more chart-focused; replace that with a
numeric/statistic-appropriate icon (e.g., NumberOutlined, FieldNumberOutlined,
or FundOutlined) from @ant-design/icons by updating the import and swapping
BarChartOutlined for the chosen icon in the AdvancedStatisticComponent
definition so the icon better reflects that this component displays statistic
values.
- Around line 178-196: The two conditional return blocks for formMode ===
'designer' duplicate rendering; consolidate them by returning a single
ConfigurableFormItem with model={{ ...passedModel, hideLabel: true }} and
valuePropName="checked", and use a single render callback that calls
renderComponent(value) (remove the differing callback signatures (value, _) vs
(value)). Keep references to formMode, ConfigurableFormItem, passedModel, and
renderComponent when making the change.
- Around line 115-119: Replace the loose any types in renderComponent and
onClickInternal: change renderComponent(value: any) to use a specific type
(e.g., value: string | number | Record<string, unknown> or a domain type
matching the data model) and type the click handler parameter instead of using
`_ : any` — e.g., onClickInternal = (_event: React.MouseEvent<HTMLElement>) => {
customEvent.onClick(value); } (or remove the unused param entirely); ensure
React types are imported and use the actual domain/interface for value if
available (or fallback to unknown | Record<string, unknown>) and keep references
to renderComponent, onClickInternal, customOnClickEventHandler, passedModel and
allData unchanged.
- Around line 203-218: The migrator repeatedly calls defaultStyles() inside the
.add<IAdvancedStatisticComponentProps>(2, (prev) => { ... }) callback; capture
the result once (e.g., const defaults = defaultStyles()) and use
defaults.valueFont, defaults.titleFont, defaults.shadow, defaults.border instead
of calling defaultStyles() multiple times so the identical object is created
only once.

In @shesha-reactjs/src/designer-components/advancedStatistic/settingsForm.ts:
- Line 757: Replace the repeated "as any" hidden-condition objects with a proper
typed interface (e.g., IHiddenCondition) and use that type for each hidden
property; define interface IHiddenCondition { _code: string; _mode: 'code' |
'value'; _value: boolean } (if no existing type fits) and update the hidden
assignments (the objects that use getSettingValue and
contexts.canvasContext?.designerDevice) to be typed as IHiddenCondition instead
of using as any so all occurrences (the hidden properties around the
advancedStatistic settingsForm) are strongly typed.
- Around line 355-359: The propertyRouteName object is using an `as any`
assertion; replace it with a concrete type describing the code-mode shape (e.g.,
an interface with _mode: 'code'|'value', _code: string, _value: string) and
cast/annotate propertyRouteName with that type instead of `any` (or reuse an
existing ICodeModeProperty type if available) so the object literal for
propertyRouteName has explicit typing and no `any` assertion.

In @shesha-reactjs/src/designer-components/advancedStatistic/utils.ts:
- Around line 25-64: The defaultStyles() factory creates a new IDefaultStyles
object on every call; replace it with a single exported frozen constant (e.g.,
DEFAULT_STYLES: IDefaultStyles) to avoid repeated allocations—construct the same
object currently returned by defaultStyles(), call Object.freeze (and freeze
nested objects if you want full immutability), export that constant and update
usages of defaultStyles() to reference DEFAULT_STYLES (or keep a tiny wrapper
that returns the constant) so callers get the static, memoized default object.
📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6b1a43e and f2b4a84.

📒 Files selected for processing (8)
  • shesha-reactjs/src/components/advancedStatistic/index.tsx
  • shesha-reactjs/src/components/advancedStatistic/interfaces.ts
  • shesha-reactjs/src/components/advancedStatistic/styles/styles.ts
  • shesha-reactjs/src/designer-components/advancedStatistic/README.md
  • shesha-reactjs/src/designer-components/advancedStatistic/index.tsx
  • shesha-reactjs/src/designer-components/advancedStatistic/settingsForm.ts
  • shesha-reactjs/src/designer-components/advancedStatistic/utils.ts
  • shesha-reactjs/src/providers/form/defaults/toolboxComponents.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CodeRabbitReview-Project-Specific-Guidelines.md)

**/*.{ts,tsx}: Eliminate the any type; use unknown type 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/designer-components/advancedStatistic/utils.ts
  • shesha-reactjs/src/components/advancedStatistic/index.tsx
  • shesha-reactjs/src/designer-components/advancedStatistic/settingsForm.ts
  • shesha-reactjs/src/components/advancedStatistic/interfaces.ts
  • shesha-reactjs/src/components/advancedStatistic/styles/styles.ts
  • shesha-reactjs/src/designer-components/advancedStatistic/index.tsx
  • shesha-reactjs/src/providers/form/defaults/toolboxComponents.ts
🧠 Learnings (15)
📓 Common learnings
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.
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3362
File: shesha-reactjs/src/designer-components/charts/settingsFormIndividual.ts:429-448
Timestamp: 2025-06-06T10:41:31.642Z
Learning: In shesha-reactjs chart settings, conditional visibility for controls is intentionally designed to show different combinations based on chart type: pie/polarArea charts show showTitle+showLegend together, while other chart types show showTitle separately. This pattern of "one shows when others doesn't show" is by design.
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4378
File: shesha-reactjs/src/designer-components/dataTable/advancedFilterButton/advancedFilterButtonComponent.tsx:48-48
Timestamp: 2026-01-08T11:19:34.113Z
Learning: In shesha-reactjs designer components (IToolboxComponent pattern), the Factory function receives a model parameter typed as the base IConfigurableFormComponent. When a component uses IToolboxComponent<SpecificProps>, a type assertion (e.g., `model as SpecificProps`) is necessary when passing model to the component to narrow from the base type to the specific prop type, even though the generic parameter is specified. This is required when SpecificProps has required properties not present in IConfigurableFormComponent.
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3461
File: shesha-reactjs/src/designer-components/charts/settingsFormIndividual.ts:171-192
Timestamp: 2025-06-26T20:01:48.838Z
Learning: In shesha-reactjs chart settings forms, duplicate input configurations may be intentionally included as part of UI/UX experiments and should be flagged for review during UI/UX review phases rather than immediately removed as errors.
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3461
File: shesha-reactjs/src/designer-components/charts/chartControl.tsx:51-51
Timestamp: 2025-06-26T20:00:58.574Z
Learning: In shesha-reactjs chart components (ChartControl), filters are intentionally passed as props rather than obtained from context. This allows the query builder and other external components to dynamically update filters. The pattern is: most chart configuration comes from useChartDataStateContext(), but filters are passed as props from the chart factories (bar.tsx, line.tsx, pie.tsx, polarArea.tsx) for dynamic updates.
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.
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4378
File: shesha-reactjs/src/designer-components/dataTable/advancedFilterButton/advancedFilterButtonComponent.tsx:61-61
Timestamp: 2026-01-08T11:22:06.111Z
Learning: In shesha-reactjs designer components (IToolboxComponent pattern), migrator functions using `.add<SpecificProps>(version, (prev) => ...)` require explicit type assertions when calling helper functions like migrateReadOnly, even though the generic parameter is specified. For example: `.add<IAdvancedFilterButtonComponentProps>(3, (prev) => migrateReadOnly(prev as IAdvancedFilterButtonComponentProps, 'inherited'))` where the cast `prev as IAdvancedFilterButtonComponentProps` must be kept.
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.
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.
📚 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/designer-components/advancedStatistic/utils.ts
  • shesha-reactjs/src/components/advancedStatistic/index.tsx
  • shesha-reactjs/src/designer-components/advancedStatistic/README.md
  • shesha-reactjs/src/components/advancedStatistic/interfaces.ts
  • shesha-reactjs/src/designer-components/advancedStatistic/index.tsx
  • shesha-reactjs/src/providers/form/defaults/toolboxComponents.ts
📚 Learning: 2025-12-11T11:57:48.398Z
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:48.398Z
Learning: In the shesha-framework's shesha-reactjs project, when using createStyles to define style hooks (e.g., export const useStyles = createStyles(...)), return an object from the callback with named keys (e.g., { columns }) and access them in components as const { styles } = useStyles(); then read properties via styles.columns. The wrapping behavior is provided by createStyles and should be followed consistently. This pattern applies to style modules under shesha-reactjs/src/designer-components (and similar styling files).

Applied to files:

  • shesha-reactjs/src/designer-components/advancedStatistic/utils.ts
  • shesha-reactjs/src/designer-components/advancedStatistic/settingsForm.ts
📚 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/designer-components/advancedStatistic/utils.ts
  • shesha-reactjs/src/components/advancedStatistic/index.tsx
  • shesha-reactjs/src/components/advancedStatistic/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/designer-components/advancedStatistic/utils.ts
  • shesha-reactjs/src/designer-components/advancedStatistic/settingsForm.ts
  • shesha-reactjs/src/designer-components/advancedStatistic/index.tsx
📚 Learning: 2026-01-08T11:22:06.111Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4378
File: shesha-reactjs/src/designer-components/dataTable/advancedFilterButton/advancedFilterButtonComponent.tsx:61-61
Timestamp: 2026-01-08T11:22:06.111Z
Learning: In shesha-reactjs designer components (IToolboxComponent pattern), migrator functions using `.add<SpecificProps>(version, (prev) => ...)` require explicit type assertions when calling helper functions like migrateReadOnly, even though the generic parameter is specified. For example: `.add<IAdvancedFilterButtonComponentProps>(3, (prev) => migrateReadOnly(prev as IAdvancedFilterButtonComponentProps, 'inherited'))` where the cast `prev as IAdvancedFilterButtonComponentProps` must be kept.

Applied to files:

  • shesha-reactjs/src/components/advancedStatistic/index.tsx
  • shesha-reactjs/src/components/advancedStatistic/interfaces.ts
  • shesha-reactjs/src/designer-components/advancedStatistic/index.tsx
  • shesha-reactjs/src/providers/form/defaults/toolboxComponents.ts
📚 Learning: 2026-01-08T11:19:34.113Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4378
File: shesha-reactjs/src/designer-components/dataTable/advancedFilterButton/advancedFilterButtonComponent.tsx:48-48
Timestamp: 2026-01-08T11:19:34.113Z
Learning: In shesha-reactjs designer components (IToolboxComponent pattern), the Factory function receives a model parameter typed as the base IConfigurableFormComponent. When a component uses IToolboxComponent<SpecificProps>, a type assertion (e.g., `model as SpecificProps`) is necessary when passing model to the component to narrow from the base type to the specific prop type, even though the generic parameter is specified. This is required when SpecificProps has required properties not present in IConfigurableFormComponent.

Applied to files:

  • shesha-reactjs/src/designer-components/advancedStatistic/settingsForm.ts
  • shesha-reactjs/src/components/advancedStatistic/interfaces.ts
  • shesha-reactjs/src/providers/form/defaults/toolboxComponents.ts
📚 Learning: 2025-06-26T20:01:48.838Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3461
File: shesha-reactjs/src/designer-components/charts/settingsFormIndividual.ts:171-192
Timestamp: 2025-06-26T20:01:48.838Z
Learning: In shesha-reactjs chart settings forms, duplicate input configurations may be intentionally included as part of UI/UX experiments and should be flagged for review during UI/UX review phases rather than immediately removed as errors.

Applied to files:

  • shesha-reactjs/src/designer-components/advancedStatistic/settingsForm.ts
  • shesha-reactjs/src/designer-components/advancedStatistic/index.tsx
  • shesha-reactjs/src/providers/form/defaults/toolboxComponents.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/advancedStatistic/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/advancedStatistic/styles/styles.ts
📚 Learning: 2026-01-07T09:12:28.286Z
Learnt from: czwe-01
Repo: shesha-io/shesha-framework PR: 4402
File: shesha-reactjs/src/components/storedFilesRendererBase/styles/styles.ts:118-128
Timestamp: 2026-01-07T09:12:28.286Z
Learning: In shesha-reactjs createStyles (using Emotion's css function), interpolating CSSProperties objects directly into css template literals (e.g., `${restContainerStyles}`) is valid and produces correct CSS output. Emotion automatically converts the object to valid CSS, unlike plain template literals where object interpolation would fail.

Applied to files:

  • shesha-reactjs/src/components/advancedStatistic/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/advancedStatistic/styles/styles.ts
📚 Learning: 2025-12-02T14:27:56.659Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4280
File: shesha-reactjs/src/components/tablePager/index.tsx:49-55
Timestamp: 2025-12-02T14:27:56.659Z
Learning: In shesha-reactjs hint Popover components (tablePagerHintPopover, quickSearchHintPopover, tableViewSelectorHintPopover), both `rootClassName` and `classNames.body` must be set to the same style class for the styles to be applied correctly to the different DOM nodes of the Ant Design Popover component.

Applied to files:

  • shesha-reactjs/src/components/advancedStatistic/styles/styles.ts
📚 Learning: 2026-01-08T11:19:26.370Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 4378
File: shesha-reactjs/src/designer-components/dataTable/advancedFilterButton/advancedFilterButtonComponent.tsx:48-48
Timestamp: 2026-01-08T11:19:26.370Z
Learning: In shesha-reactjs designer components (IToolboxComponent pattern), when a component uses IToolboxComponent<SpecificProps>, pass the generic-parameter-specific props safely by narrowing the base model type. If the Factory function receives a model typed as IConfigurableFormComponent and you render a child component expecting IToolboxComponent<SpecificProps>, you may need to cast the model (e.g., model as SpecificProps) to satisfy required properties present only on SpecificProps. This pattern is necessary when SpecificProps has required fields not present in IConfigurableFormComponent. Apply this guidance to all similar TSX files under shesha-reactjs/src/designer-components (and related designer components) where a base type is extended by a more specific prop type.

Applied to files:

  • shesha-reactjs/src/designer-components/advancedStatistic/index.tsx
📚 Learning: 2025-06-06T10:41:31.642Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3362
File: shesha-reactjs/src/designer-components/charts/settingsFormIndividual.ts:429-448
Timestamp: 2025-06-06T10:41:31.642Z
Learning: In shesha-reactjs chart settings, conditional visibility for controls is intentionally designed to show different combinations based on chart type: pie/polarArea charts show showTitle+showLegend together, while other chart types show showTitle separately. This pattern of "one shows when others doesn't show" is by design.

Applied to files:

  • shesha-reactjs/src/providers/form/defaults/toolboxComponents.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 (12)
shesha-reactjs/src/designer-components/advancedStatistic/README.md (1)

1-205: LGTM! Well-structured component documentation.

The README provides comprehensive coverage of features, configuration properties, usage examples, and visual layout diagrams. The migration versioning notes are helpful for future maintenance.

shesha-reactjs/src/providers/form/defaults/toolboxComponents.ts (2)

46-46: LGTM!

Import follows the existing pattern and is correctly placed adjacent to the related Statistic import.


149-149: LGTM!

The AdvancedStatistic component is appropriately placed in the Data display group alongside the existing Statistic component.

shesha-reactjs/src/components/advancedStatistic/interfaces.ts (1)

1-59: LGTM! Well-typed interfaces.

The interfaces are properly defined with appropriate optional properties for a highly configurable component. The use of string literal union for align and proper React.MouseEvent typing follows TypeScript best practices.

shesha-reactjs/src/components/advancedStatistic/styles/styles.ts (1)

87-99: LGTM!

The styles hook correctly follows the createStyles pattern and returns all named style properties as expected.

shesha-reactjs/src/components/advancedStatistic/index.tsx (4)

8-16: LGTM!

The formatValue helper correctly handles undefined/null values and applies precision formatting for numbers.


152-154: Verify the fallback to '0' is intentional.

When value is an empty string, formattedValue will also be empty, and the fallback renders '0'. If displaying nothing is desired when the value is intentionally empty, consider adjusting the fallback logic.


36-119: LGTM!

The memoization strategy is well-implemented with correct dependency arrays. The style computation and merging logic using removeUndefinedProps is clean and consistent.


127-180: LGTM!

The conditional rendering logic for icons, titles, prefix, and suffix is correctly implemented with proper null checks.

shesha-reactjs/src/designer-components/advancedStatistic/settingsForm.ts (1)

1-6: LGTM on imports and structure.

The imports are appropriate for the settings form markup factory pattern. The overall structure follows established patterns in the codebase.

shesha-reactjs/src/designer-components/advancedStatistic/index.tsx (1)

21-69: LGTM on interface definitions.

The IIconConfig, IPrefixSuffixConfig, and IAdvancedStatisticComponentProps interfaces are well-structured with appropriate optional properties for the component's configurability.

shesha-reactjs/src/designer-components/advancedStatistic/utils.ts (1)

46-55: The border structure is correct. The border.border.border.all nesting properly follows the type definitions: IDefaultStyles.border contains border: IBorderValue, which itself has an optional border property containing all, top, right, bottom, left, and middle sub-properties. There is no type mismatch or structural issue.

Likely an incorrect or invalid review comment.

Comment on lines +121 to +126
return (
<div
className={classNames(styles.advancedStatisticContainer)}
style={containerStyle}
onClick={onClick}
>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Simplify: classNames wrapper is unnecessary for a single class.

Since only one class is being applied, classNames can be removed.

Suggested simplification
    <div
-     className={classNames(styles.advancedStatisticContainer)}
+     className={styles.advancedStatisticContainer}
      style={containerStyle}
      onClick={onClick}
    >
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return (
<div
className={classNames(styles.advancedStatisticContainer)}
style={containerStyle}
onClick={onClick}
>
return (
<div
className={styles.advancedStatisticContainer}
style={containerStyle}
onClick={onClick}
>
🤖 Prompt for AI Agents
In @shesha-reactjs/src/components/advancedStatistic/index.tsx around lines 121 -
126, The JSX return uses classNames(styles.advancedStatisticContainer)
unnecessarily; replace the classNames call with the plain class reference by
setting className to styles.advancedStatisticContainer (remove the classNames
import/use) in the component that renders this div so the element uses
className={styles.advancedStatisticContainer} and delete the unused classNames
reference/import if no other usage remains.

Comment on lines +3 to +14
export const useStyles = createStyles(({ css, cx, token }) => {
const advancedStatisticContainer = cx("sha-advanced-statistic", css`
display: flex;
align-items: center;
background: white;
border-radius: 8px;
box-shadow: 0 7px 30px -10px rgba(150, 170, 180, 0.5);
padding: ${sheshaStyles.paddingMD}px;
height: 100%;
position: relative;
box-sizing: border-box;
`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Use theme tokens instead of hardcoded colors for dark mode support.

The hardcoded background: white will break in dark mode themes. Consider using the theme token for consistent theming.

Suggested fix
  const advancedStatisticContainer = cx("sha-advanced-statistic", css`
    display: flex;
    align-items: center;
-   background: white;
+   background: ${token.colorBgContainer};
    border-radius: 8px;
    box-shadow: 0 7px 30px -10px rgba(150, 170, 180, 0.5);
    padding: ${sheshaStyles.paddingMD}px;
    height: 100%;
    position: relative;
    box-sizing: border-box;
  `);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const useStyles = createStyles(({ css, cx, token }) => {
const advancedStatisticContainer = cx("sha-advanced-statistic", css`
display: flex;
align-items: center;
background: white;
border-radius: 8px;
box-shadow: 0 7px 30px -10px rgba(150, 170, 180, 0.5);
padding: ${sheshaStyles.paddingMD}px;
height: 100%;
position: relative;
box-sizing: border-box;
`);
export const useStyles = createStyles(({ css, cx, token }) => {
const advancedStatisticContainer = cx("sha-advanced-statistic", css`
display: flex;
align-items: center;
background: ${token.colorBgContainer};
border-radius: 8px;
box-shadow: 0 7px 30px -10px rgba(150, 170, 180, 0.5);
padding: ${sheshaStyles.paddingMD}px;
height: 100%;
position: relative;
box-sizing: border-box;
`);
🤖 Prompt for AI Agents
In @shesha-reactjs/src/components/advancedStatistic/styles/styles.ts around
lines 3 - 14, The container currently uses a hardcoded background ("background:
white") which breaks dark mode; update the useStyles/createStyles block
(specifically the advancedStatisticContainer definition inside useStyles) to use
the provided theme token instead (e.g., token.colorBgContainer or another
appropriate token like token.colorBgLayout) so the background adapts to
theme/dark mode, and remove the hardcoded white value; ensure you reference the
token param already available in createStyles and pick a token consistent with
other components.

Comment on lines +40 to +52
const titleTop = css`
width: 100%;
margin-bottom: ${sheshaStyles.paddingSM}px;
font-size: 14px;
color: rgba(0, 0, 0, 0.65);
`;

const titleBottom = css`
width: 100%;
margin-top: ${sheshaStyles.paddingSM}px;
font-size: 14px;
color: rgba(0, 0, 0, 0.65);
`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Use theme tokens for title colors.

The hardcoded rgba(0, 0, 0, 0.65) color won't adapt to dark themes. Use token.colorTextSecondary for better theme consistency.

Suggested fix
  const titleTop = css`
    width: 100%;
    margin-bottom: ${sheshaStyles.paddingSM}px;
    font-size: 14px;
-   color: rgba(0, 0, 0, 0.65);
+   color: ${token.colorTextSecondary};
  `;

  const titleBottom = css`
    width: 100%;
    margin-top: ${sheshaStyles.paddingSM}px;
    font-size: 14px;
-   color: rgba(0, 0, 0, 0.65);
+   color: ${token.colorTextSecondary};
  `;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const titleTop = css`
width: 100%;
margin-bottom: ${sheshaStyles.paddingSM}px;
font-size: 14px;
color: rgba(0, 0, 0, 0.65);
`;
const titleBottom = css`
width: 100%;
margin-top: ${sheshaStyles.paddingSM}px;
font-size: 14px;
color: rgba(0, 0, 0, 0.65);
`;
const titleTop = css`
width: 100%;
margin-bottom: ${sheshaStyles.paddingSM}px;
font-size: 14px;
color: ${token.colorTextSecondary};
`;
const titleBottom = css`
width: 100%;
margin-top: ${sheshaStyles.paddingSM}px;
font-size: 14px;
color: ${token.colorTextSecondary};
`;
🤖 Prompt for AI Agents
In @shesha-reactjs/src/components/advancedStatistic/styles/styles.ts around
lines 40 - 52, The title styles titleTop and titleBottom use a hardcoded color
rgba(0,0,0,0.65) which won't adapt to dark themes; update both definitions to
use the theme token (token.colorTextSecondary) instead and ensure the token is
obtained via the theme hook or context (e.g., useToken or theme tokens) in the
styles module so the color resolves from the current theme.

Comment on lines +71 to +76
const AdvancedStatisticComponent: IToolboxComponent<IAdvancedStatisticComponentProps> = {
type: 'advancedStatistic',
name: 'Advanced Statistic',
icon: <BarChartOutlined />,
isInput: true,
isOutput: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider using a more fitting icon for a statistic component.

BarChartOutlined is used for the icon, but this component displays statistic values rather than charts. Consider using NumberOutlined, FieldNumberOutlined, or FundOutlined from @ant-design/icons for better semantic alignment.

🤖 Prompt for AI Agents
In @shesha-reactjs/src/designer-components/advancedStatistic/index.tsx around
lines 71 - 76, The component AdvancedStatisticComponent currently uses
BarChartOutlined as its icon which is more chart-focused; replace that with a
numeric/statistic-appropriate icon (e.g., NumberOutlined, FieldNumberOutlined,
or FundOutlined) from @ant-design/icons by updating the import and swapping
BarChartOutlined for the chosen icon in the AdvancedStatisticComponent
definition so the icon better reflects that this component displays statistic
values.

Comment on lines +115 to +119
const renderComponent = (value: any): React.ReactElement => {
const customEvent = customOnClickEventHandler(passedModel, allData);
const onClickInternal = (_: any): void => {
customEvent.onClick(value);
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Replace any types with proper typing.

Per coding guidelines, eliminate any type. The value parameter and the unused _ parameter in onClickInternal should be properly typed.

♻️ Proposed fix
-    const renderComponent = (value: any): React.ReactElement => {
+    const renderComponent = (value: number | string | undefined): React.ReactElement => {
       const customEvent = customOnClickEventHandler(passedModel, allData);
-      const onClickInternal = (_: any): void => {
+      const onClickInternal = (): void => {
         customEvent.onClick(value);
       };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const renderComponent = (value: any): React.ReactElement => {
const customEvent = customOnClickEventHandler(passedModel, allData);
const onClickInternal = (_: any): void => {
customEvent.onClick(value);
};
const renderComponent = (value: number | string | undefined): React.ReactElement => {
const customEvent = customOnClickEventHandler(passedModel, allData);
const onClickInternal = (): void => {
customEvent.onClick(value);
};
🤖 Prompt for AI Agents
In @shesha-reactjs/src/designer-components/advancedStatistic/index.tsx around
lines 115 - 119, Replace the loose any types in renderComponent and
onClickInternal: change renderComponent(value: any) to use a specific type
(e.g., value: string | number | Record<string, unknown> or a domain type
matching the data model) and type the click handler parameter instead of using
`_ : any` — e.g., onClickInternal = (_event: React.MouseEvent<HTMLElement>) => {
customEvent.onClick(value); } (or remove the unused param entirely); ensure
React types are imported and use the actual domain/interface for value if
available (or fallback to unknown | Record<string, unknown>) and keep references
to renderComponent, onClickInternal, customOnClickEventHandler, passedModel and
allData unchanged.

Comment on lines +178 to +196
if (formMode === 'designer') {
return (
<ConfigurableFormItem
model={{ ...passedModel, hideLabel: true }}
valuePropName="checked"
>
{(value, _) => renderComponent(value)}
</ConfigurableFormItem>
);
}

return (
<ConfigurableFormItem
model={{ ...passedModel, hideLabel: true }}
valuePropName="checked"
>
{(value) => renderComponent(value)}
</ConfigurableFormItem>
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Remove duplicate rendering logic.

The designer and non-designer mode blocks are nearly identical. The only difference is the callback signature on line 184 (value, _) vs line 194 (value), but both call renderComponent(value). This can be simplified to a single return.

♻️ Proposed fix
-    if (formMode === 'designer') {
-      return (
-        <ConfigurableFormItem
-          model={{ ...passedModel, hideLabel: true }}
-          valuePropName="checked"
-        >
-          {(value, _) => renderComponent(value)}
-        </ConfigurableFormItem>
-      );
-    }
-
     return (
       <ConfigurableFormItem
         model={{ ...passedModel, hideLabel: true }}
         valuePropName="checked"
       >
         {(value) => renderComponent(value)}
       </ConfigurableFormItem>
     );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (formMode === 'designer') {
return (
<ConfigurableFormItem
model={{ ...passedModel, hideLabel: true }}
valuePropName="checked"
>
{(value, _) => renderComponent(value)}
</ConfigurableFormItem>
);
}
return (
<ConfigurableFormItem
model={{ ...passedModel, hideLabel: true }}
valuePropName="checked"
>
{(value) => renderComponent(value)}
</ConfigurableFormItem>
);
return (
<ConfigurableFormItem
model={{ ...passedModel, hideLabel: true }}
valuePropName="checked"
>
{(value) => renderComponent(value)}
</ConfigurableFormItem>
);
🤖 Prompt for AI Agents
In @shesha-reactjs/src/designer-components/advancedStatistic/index.tsx around
lines 178 - 196, The two conditional return blocks for formMode === 'designer'
duplicate rendering; consolidate them by returning a single ConfigurableFormItem
with model={{ ...passedModel, hideLabel: true }} and valuePropName="checked",
and use a single render callback that calls renderComponent(value) (remove the
differing callback signatures (value, _) vs (value)). Keep references to
formMode, ConfigurableFormItem, passedModel, and renderComponent when making the
change.

Comment on lines +203 to +218
.add<IAdvancedStatisticComponentProps>(2, (prev) => {
const styles = {
containerStyle: prev?.containerStyle,
valueFont: defaultStyles().valueFont,
valueStyle: prev?.valueStyle,
titleFont: defaultStyles().titleFont,
shadow: defaultStyles().shadow,
border: defaultStyles().border,
};

return {
...prev,
desktop: { ...styles },
tablet: { ...styles },
mobile: { ...styles },
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Optimize repeated defaultStyles() calls.

The defaultStyles() function is called 4 times in the migrator, creating identical objects each time. Store the result once for efficiency.

♻️ Proposed fix
       .add<IAdvancedStatisticComponentProps>(2, (prev) => {
+        const defaults = defaultStyles();
         const styles = {
           containerStyle: prev?.containerStyle,
-          valueFont: defaultStyles().valueFont,
+          valueFont: defaults.valueFont,
           valueStyle: prev?.valueStyle,
-          titleFont: defaultStyles().titleFont,
-          shadow: defaultStyles().shadow,
-          border: defaultStyles().border,
+          titleFont: defaults.titleFont,
+          shadow: defaults.shadow,
+          border: defaults.border,
         };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
.add<IAdvancedStatisticComponentProps>(2, (prev) => {
const styles = {
containerStyle: prev?.containerStyle,
valueFont: defaultStyles().valueFont,
valueStyle: prev?.valueStyle,
titleFont: defaultStyles().titleFont,
shadow: defaultStyles().shadow,
border: defaultStyles().border,
};
return {
...prev,
desktop: { ...styles },
tablet: { ...styles },
mobile: { ...styles },
};
.add<IAdvancedStatisticComponentProps>(2, (prev) => {
const defaults = defaultStyles();
const styles = {
containerStyle: prev?.containerStyle,
valueFont: defaults.valueFont,
valueStyle: prev?.valueStyle,
titleFont: defaults.titleFont,
shadow: defaults.shadow,
border: defaults.border,
};
return {
...prev,
desktop: { ...styles },
tablet: { ...styles },
mobile: { ...styles },
};
🤖 Prompt for AI Agents
In @shesha-reactjs/src/designer-components/advancedStatistic/index.tsx around
lines 203 - 218, The migrator repeatedly calls defaultStyles() inside the
.add<IAdvancedStatisticComponentProps>(2, (prev) => { ... }) callback; capture
the result once (e.g., const defaults = defaultStyles()) and use
defaults.valueFont, defaults.titleFont, defaults.shadow, defaults.border instead
of calling defaultStyles() multiple times so the identical object is created
only once.

Comment on lines +355 to +359
propertyRouteName: {
_mode: "code",
_code: " return contexts.canvasContext?.designerDevice || 'desktop';",
_value: "",
} as any,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Avoid as any type assertion; define a proper type.

Per coding guidelines, eliminate the any type. The propertyRouteName object has a specific structure (_mode, _code, _value) that should be typed explicitly.

♻️ Proposed fix

Define or use an existing type for code-mode properties:

// If not already defined elsewhere, add this type
interface ICodeModeProperty {
  _mode: 'code' | 'value';
  _code: string;
  _value: string;
}

// Then use it instead of `as any`:
propertyRouteName: {
  _mode: "code",
  _code: "    return contexts.canvasContext?.designerDevice || 'desktop';",
  _value: "",
} as ICodeModeProperty,
🤖 Prompt for AI Agents
In @shesha-reactjs/src/designer-components/advancedStatistic/settingsForm.ts
around lines 355 - 359, The propertyRouteName object is using an `as any`
assertion; replace it with a concrete type describing the code-mode shape (e.g.,
an interface with _mode: 'code'|'value', _code: string, _value: string) and
cast/annotate propertyRouteName with that type instead of `any` (or reuse an
existing ICodeModeProperty type if available) so the object literal for
propertyRouteName has explicit typing and no `any` assertion.

hideLabel: true,
jsSetting: false,
}],
hidden: { _code: 'return getSettingValue(data[`${contexts.canvasContext?.designerDevice || "desktop"}`]?.background?.type) !== "color";', _mode: 'code', _value: false } as any,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Multiple as any assertions for hidden conditions violate type safety guidelines.

The hidden conditions on lines 757, 770, 783, 795, 800, 815, and 848 all use as any for the same code-mode pattern. Define a shared type for these hidden condition objects to eliminate any usage.

♻️ Proposed fix
// Define a type for hidden conditions (if not already available)
interface IHiddenCondition {
  _code: string;
  _mode: 'code' | 'value';
  _value: boolean;
}

// Then replace all occurrences like:
hidden: { 
  _code: 'return getSettingValue(data[`${contexts.canvasContext?.designerDevice || "desktop"}`]?.background?.type) !== "color";', 
  _mode: 'code', 
  _value: false 
} as IHiddenCondition,
🤖 Prompt for AI Agents
In @shesha-reactjs/src/designer-components/advancedStatistic/settingsForm.ts at
line 757, Replace the repeated "as any" hidden-condition objects with a proper
typed interface (e.g., IHiddenCondition) and use that type for each hidden
property; define interface IHiddenCondition { _code: string; _mode: 'code' |
'value'; _value: boolean } (if no existing type fits) and update the hidden
assignments (the objects that use getSettingValue and
contexts.canvasContext?.designerDevice) to be typed as IHiddenCondition instead
of using as any so all occurrences (the hidden properties around the
advancedStatistic settingsForm) are strongly typed.

Comment on lines +25 to +64
export const defaultStyles = (): IDefaultStyles => {
return {
valueFont: {
type: 'Roboto',
size: 32,
weight: '600',
color: '#1890ff',
},
titleFont: {
type: 'Roboto',
size: 14,
weight: '400',
color: 'rgba(0, 0, 0, 0.65)',
},
shadow: {
offsetX: 0,
offsetY: 7,
blurRadius: 30,
spreadRadius: -10,
color: 'rgba(150, 170, 180, 0.5)',
},
border: {
border: {
border: {
all: {
width: 1,
color: '#d9d9d9',
style: 'solid',
},
},
},
borderRadius: {
borderTopLeftRadius: '8px',
borderTopRightRadius: '8px',
borderBottomLeftRadius: '8px',
borderBottomRightRadius: '8px',
},
},
};
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider memoizing or using a constant for static defaults.

The defaultStyles() function creates a new object on every call. If these defaults are truly static and called frequently (e.g., in migrators for each component), consider exporting a frozen constant to avoid unnecessary object allocations.

♻️ Optional: Use a frozen constant
-export const defaultStyles = (): IDefaultStyles => {
-  return {
+const DEFAULT_STYLES: IDefaultStyles = Object.freeze({
     valueFont: {
       type: 'Roboto',
       size: 32,
       weight: '600',
       color: '#1890ff',
     },
     // ... rest of properties
-  };
-};
+}) as IDefaultStyles;
+
+export const defaultStyles = (): IDefaultStyles => DEFAULT_STYLES;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const defaultStyles = (): IDefaultStyles => {
return {
valueFont: {
type: 'Roboto',
size: 32,
weight: '600',
color: '#1890ff',
},
titleFont: {
type: 'Roboto',
size: 14,
weight: '400',
color: 'rgba(0, 0, 0, 0.65)',
},
shadow: {
offsetX: 0,
offsetY: 7,
blurRadius: 30,
spreadRadius: -10,
color: 'rgba(150, 170, 180, 0.5)',
},
border: {
border: {
border: {
all: {
width: 1,
color: '#d9d9d9',
style: 'solid',
},
},
},
borderRadius: {
borderTopLeftRadius: '8px',
borderTopRightRadius: '8px',
borderBottomLeftRadius: '8px',
borderBottomRightRadius: '8px',
},
},
};
};
const DEFAULT_STYLES: IDefaultStyles = Object.freeze({
valueFont: {
type: 'Roboto',
size: 32,
weight: '600',
color: '#1890ff',
},
titleFont: {
type: 'Roboto',
size: 14,
weight: '400',
color: 'rgba(0, 0, 0, 0.65)',
},
shadow: {
offsetX: 0,
offsetY: 7,
blurRadius: 30,
spreadRadius: -10,
color: 'rgba(150, 170, 180, 0.5)',
},
border: {
border: {
border: {
all: {
width: 1,
color: '#d9d9d9',
style: 'solid',
},
},
},
borderRadius: {
borderTopLeftRadius: '8px',
borderTopRightRadius: '8px',
borderBottomLeftRadius: '8px',
borderBottomRightRadius: '8px',
},
},
}) as IDefaultStyles;
export const defaultStyles = (): IDefaultStyles => DEFAULT_STYLES;
🤖 Prompt for AI Agents
In @shesha-reactjs/src/designer-components/advancedStatistic/utils.ts around
lines 25 - 64, The defaultStyles() factory creates a new IDefaultStyles object
on every call; replace it with a single exported frozen constant (e.g.,
DEFAULT_STYLES: IDefaultStyles) to avoid repeated allocations—construct the same
object currently returned by defaultStyles(), call Object.freeze (and freeze
nested objects if you want full immutability), export that constant and update
usages of defaultStyles() to reference DEFAULT_STYLES (or keep a tiny wrapper
that returns the constant) so callers get the static, memoized default object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant