Skip to content

Conversation

@betterdancing
Copy link
Contributor

@betterdancing betterdancing commented Jan 21, 2026

English | 简体中文

PR

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our Commit Message Guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • Built its own designer, fully self-validated

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

Background and solution

What is the current behavior?

图表组件的option配置面板现在是作为一个整体交给用户配置,体验很不好

Issue Number: N/A

What is the new behavior?

将options里各个属性拆分成单独的配置属性由用户配置

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Summary by CodeRabbit

  • New Features
    • Expanded histogram chart options for detailed styling and data and presentation control (colors, series, axes, legend, zoom, tooltips, padding, themes, etc.).
    • Added chart lifecycle events (onReady, onReadyOnce) for render callbacks.
    • Introduced a nested-property configurator component for editing grouped/nested properties in the UI and made it available from the configurator package.

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

@github-actions github-actions bot added the enhancement New feature or request label Jan 21, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 21, 2026

Walkthrough

Adds a detailed properties schema and two lifecycle events to TinyHuichartsHistogram in the demo bundle, and introduces a new NestedPropertyConfigurator Vue component which is exported from the configurator package.

Changes

Cohort / File(s) Summary
Chart Configuration Schema
designer-demo/public/mock/bundle.json
Adds a comprehensive properties array under TinyHuichartsHistogram.options with many configurable fields (color, data, dataRules, dataZoom, direction, itemStyle, label, legend, lineDataName, markline, padding, theme, tooltip, type, xAxis, yAxis, etc.), including widget metadata, defaults, and descriptions; adds events entries onReady and onReadyOnce.
Nested Property Configurator
packages/configurator/src/nested-property-configurator/NestedPropertyConfigurator.vue
Adds new Vue SFC NestedPropertyConfigurator that renders a CodeConfigurator, iterates meta.properties[0].content to render config items, initializes/synchronizes per-property model values, emits update:modelValue via itemChange, and includes scoped styles for layout/scrolling.
Configurator Exports
packages/configurator/src/index.ts
Imports and re-exports NestedPropertyConfigurator, adding it to the configurator package public API.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I hopped through JSON fields with glee,
Nesting keys and widgets under a tree.
I emit a value, tidy and neat,
Configs in order, every sheet—
A rabbit's cheer for code complete. 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 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.
Title check ✅ Passed The pull request title accurately describes the main change: adding a NestedObjectConfigurator component for chart components and updating the chart bundle configuration with granular property options.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

🧪 Unit Test Generation v2 is now available!

We have significantly improved our unit test generation capabilities.

To enable: Add this to your .coderabbit.yaml configuration:

reviews:
  finishing_touches:
    unit_tests:
      enabled: true

Try it out by using the @coderabbitai generate unit tests command on your code files or under ✨ Finishing Touches on the walkthrough!

Have feedback? Share your thoughts on our Discord thread!


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: 4

🤖 Fix all issues with AI agents
In
`@packages/configurator/src/object-depth-configurator/ObjectDepthConfigurator.vue`:
- Line 8: The template references an undefined variable via
:data-group-index="index" in ObjectDepthConfigurator.vue; add a prop named index
to the component's props (or define it in setup) or remove/replace the binding
with the correct existing value. Specifically, if ConfigItem consumers expect a
numeric group index, add an index prop (e.g., props: { index: { type: Number,
required: true } }) to the ObjectDepthConfigurator component and use that prop
in :data-group-index, otherwise change the attribute to use the correct existing
variable or omit it.
- Around line 27-30: The prop definition for meta in ObjectDepthConfigurator.vue
uses a default arrow function that returns undefined; update the meta prop's
default to return an empty object (e.g., change the default factory to return {}
instead of using a block with no return) so that accesses like
props.meta.properties or props.meta.widget won't throw; locate the meta prop in
the component options (meta: { type: Object, default: ... }) and replace the
default implementation with a function that returns an empty object.
- Around line 34-47: The computed property properties is mutating objects from
props.meta.properties[0].content (setting item.widget.props.modelValue) and also
accesses props.meta.widget.props?.modelValue without ensuring props.meta.widget
exists; change properties to produce a new array (use map) and deep-clone each
item before assigning modelValue so you don't mutate the original props, and
guard access to props.meta.widget (e.g., read
props.meta.widget?.props?.modelValue) when retrieving propsModelValue; update
references to the computed properties, props.meta.properties,
props.meta.widget.props.modelValue, and item.widget.props.modelValue while
making these changes.
- Around line 48-50: itemChange currently reads
props.meta.widget.props.modelValue without null checks which can throw if
meta/widget/props is undefined; update itemChange to safely access the nested
value (use optional chaining on props.meta?.widget?.props?.modelValue) and
default to an empty object before spreading, then emit the update as before
(reference itemChange and the emit('update:modelValue', ...) call).
🧹 Nitpick comments (4)
packages/configurator/src/object-depth-configurator/ObjectDepthConfigurator.vue (1)

3-6: Redundant :key on nested element.

The :key="idx" on line 5 is redundant since the parent div already has :key="idx". The key is only needed on the element with v-for.

Proposed fix
     <div v-for="(data, idx) in properties" :key="idx" class="meta-config-item">
       <config-item
-        :key="idx"
         :property="data"
         :data-prop-index="idx"
designer-demo/public/mock/bundle.json (3)

4115-4130: Extraneous language prop in SelectConfigurator.

The language: "json" prop on Line 4118 appears to be copy-paste residue from CodeConfigurator. SelectConfigurator does not use a language prop.

🧹 Suggested cleanup
                            "widget": {
                              "component": "SelectConfigurator",
                              "props": {
-                               "language": "json",
                                "options": [
                                  {
                                    "label": "vertical",
                                    "value": "vertical"
                                  },
                                  {
                                    "label": "horizontal",
                                    "value": "horizontal"
                                  }
                                ]
                              }
                            },

4143-4155: Mixed line endings in defaultValue.

The defaultValue on Line 4146 uses \r\n initially but ends with just \n, creating inconsistent line endings. This could cause display or parsing inconsistencies.

🧹 Suggested fix for consistent line endings
-                           "defaultValue": "{ \r\n\"barWidth\": 8, \r\n\"barGap\": \"60%\"\n}",
+                           "defaultValue": "{\n  \"barWidth\": 8,\n  \"barGap\": \"60%\"\n}",

4066-4103: Consider relaxing required: true for optional chart features.

Properties like dataRules, dataZoom, markline, legend, label, tooltip, and lineDataName are typically optional chart enhancements. Marking all of them as required: true may force users to provide values for features they don't intend to use, reducing configuration flexibility.

Consider setting "required": false for genuinely optional properties.

@hexqi hexqi added this to the v2.10.0 milestone Jan 29, 2026
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: 3

🤖 Fix all issues with AI agents
In `@designer-demo/public/mock/bundle.json`:
- Around line 4115-4130: Remove the unnecessary "language": "json" property from
the props object of any widget where "component": "SelectConfigurator" (e.g.,
the widget shown with "component": "SelectConfigurator"); locate the props for
SelectConfigurator and delete the language key so only relevant props like
"options" remain, leaving CodeConfigurator usages untouched.

In
`@packages/configurator/src/nested-property-configurator/NestedPropertyConfigurator.vue`:
- Line 22: The component's declared name "ObjectDepthConfigurator" in the
component options should match the file and export name; change the Vue
component option "name" to "NestedPropertyConfigurator" (ensure the object with
name: 'ObjectDepthConfigurator' is updated to name:
'NestedPropertyConfigurator') and then search for any references to
"ObjectDepthConfigurator" (tests, imports, or devtools usage) and update them to
"NestedPropertyConfigurator" so the runtime name, filename, and export remain
consistent.
- Around line 34-46: The computed properties getter (properties) is still
mutating nested objects because the shallow array copy ([...]) doesn't clone
item objects or their nested widget/props; also access to
props.meta.widget.props may throw if props.meta.widget is undefined. Fix by
replacing the forEach mutation with a map that returns brand-new item objects
(spread the item, and also spread item.widget and item.widget.props) and set
widget.props.modelValue on the new object only; guard access to
props.meta.widget before reading props.meta.widget.props (e.g., derive
propsModelValue only when props.meta?.widget exists). Ensure the computed
returns the mapped array of new objects so parent state isn't mutated.
🧹 Nitpick comments (2)
designer-demo/public/mock/bundle.json (2)

4143-4155: Consider using \n instead of \r\n in defaultValue.

The defaultValue for itemStyle contains Windows-style line endings (\r\n). This could cause inconsistent display or parsing issues across different platforms. Using \n alone is more portable.

🧹 Suggested fix
-                           "defaultValue": "{ \r\n\"barWidth\": 8, \r\n\"barGap\": \"60%\"\n}",
+                           "defaultValue": "{\n  \"barWidth\": 8,\n  \"barGap\": \"60%\"\n}",

4066-4103: Consider relaxing required: true for optional chart properties.

Properties like dataRules, dataZoom, and markline are typically optional chart configurations that users may not need to specify. Marking them all as required: true could create unnecessary friction in the configurator UI or cause validation issues when these optional features aren't needed.

Also applies to: 4220-4238

hexqi
hexqi previously approved these changes Jan 30, 2026
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: 1

🤖 Fix all issues with AI agents
In
`@packages/configurator/src/nested-property-configurator/NestedPropertyConfigurator.vue`:
- Around line 45-50: props.meta.widget is being dereferenced without a guard
causing setup/computed to throw when widget is undefined; update usages in the
setup: initialize bindValue from props.meta?.widget?.props?.modelValue (or
fallback to undefined) and change the properties computed to reference
props.meta?.widget (use optional chaining) where needed so you never access
.props on an undefined widget; specifically update the bindValue assignment and
the computed named properties to use props.meta?.widget?.props?.modelValue and
fallback defaults (e.g., undefined or []) to avoid runtime errors.

@hexqi hexqi changed the title Feat/chart options configurator: The options configuration panel is added for chart components. feat(configurator): add NestedObjectConfigurator for chart components and update chart bundle Jan 30, 2026
@hexqi hexqi merged commit c31ec27 into opentiny:develop Jan 30, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants