Feat/schema css object: The style class is presented as an object in the page schema. #1749
Conversation
…into feat/schema-css-object
WalkthroughEnable deep watching of schema CSS, add object-to-string CSS normalization utility, propagate normalization across style handling (initStyle, generator, settings, useStyle), and change generateCssString to return a CSS object instead of a string. Changes
Sequence Diagram(s)sequenceDiagram
participant Editor as Editor/UI
participant Settings as Styles Component
participant Utils as objectCssToString
participant Parser as CSS Parser
participant Renderer as RenderMain
participant Generator as Vue Generator
Editor->>Settings: edit/save CSS (string or object)
Settings->>Utils: objectCssToString(content)
Utils-->>Settings: css string
Settings->>Parser: parser(css string)
Parser-->>Settings: parsed object/state
Settings->>Renderer: update schema.css (object)
Renderer->>Renderer: watcher (deep: true) triggers setPageCss
Renderer->>Generator: initStyle(key, cssObject)
Generator->>Utils: objectCssToString(cssObject)
Utils-->>Generator: css string for SFC output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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: 2
🤖 Fix all issues with AI agents
In `@packages/settings/styles/src/components/classNamesContainer/index.vue`:
- Around line 450-474: The custom regex parser cssStringToObject is dropping
complex CSS constructs (at-rules like `@media/`@keyframes, multiple selectors, and
property values containing semicolons or braces); replace its use with the
existing CSS parser/stringify utilities used in updateGlobalStyle() so the code
round-trips without loss — locate cssStringToObject and change the logic to
parse the input using the shared parser API and to serialize using the matching
stringify utility (following updateGlobalStyle()’s pattern) so `@rules`, grouped
selectors and literal property values (e.g., content: ";") are preserved.
In `@packages/utils/src/utils/index.ts`:
- Around line 462-479: The function obJectCssToString should guard against css
being null or undefined (and non-object) to avoid runtime crashes; at the top of
obJectCssToString check if css is null/undefined and return an empty string (or
if typeof css === 'string' keep returning it), and when iterating ensure
properties and properties[property] are accessed only if properties is truthy
(skip null/undefined property objects) so the for-in loops never run on null;
update obJectCssToString accordingly to early-return for falsy/non-object inputs
and to safely handle missing nested property objects.
🧹 Nitpick comments (3)
packages/settings/styles/src/components/classNamesContainer/index.vue (1)
476-483: RenamecssStringto reflect it’s now an object.
Minor clarity improvement.✏️ Suggested rename
- const cssString = cssStringToObject(content) + const cssObject = cssStringToObject(content) ... - updateSchema({ css: cssString }) + updateSchema({ css: cssObject })packages/canvas/render/src/material-function/page-getter.ts (2)
5-7: Unconventional function naming:obJectCssToStringThe utility function name uses unusual casing (
obJectwith capital 'J'). Consider renaming toobjectCssToStringfor consistency with standard camelCase conventions. This would need to be updated inpackages/utils/src/utils/index.tswhere it's defined.
21-39: Integration looks correct; consider a more specific type.The changes correctly handle both string and object CSS content:
- Early return guards against falsy content
- Conversion via
obJectCssToStringhappens beforehandleScopedCss(which expects a string)The type
objectis quite broad. For improved type safety, consider defining a more specific type:type CssObject = Record<string, Record<string, string>> export function initStyle(key: string, content: string | CssObject)This aligns with the structure expected by
obJectCssToString(selector → properties → values).
packages/settings/styles/src/components/classNamesContainer/index.vue
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/plugins/page/src/composable/usePage.ts`:
- Around line 134-140: When materialsOptions.useBaseStyle is true the code
accesses materialsOptions.blockBaseStyle and materialsOptions.componentBaseStyle
without null checks; add guards so you only reference .className and .style when
blockBaseStyle/componentBaseStyle are defined. Update the block that builds
cssObject (the parseStyle calls and computed keys) to check existence of
materialsOptions.blockBaseStyle and materialsOptions.componentBaseStyle (and
their .className/.style) before adding those entries to cssObject, falling back
to skipping the entry if missing. Keep parseStyle and cssObject merging logic
unchanged except for these presence checks.
- Around line 119-129: The parseStyle function must robustly handle
empty/whitespace items, keys/values with extra spaces, and values that contain
colons; update parseStyle to first trim the incoming styleString and skip any
empty items, for each non-empty item find the first colon index (instead of
.split) to separate key and value, trim both key and value, and only assign when
the colon exists and value is defined (skip malformed entries) so values like
url(http://...) or data:... are preserved and whitespace-only segments don't
cause .trim() on undefined.
🧹 Nitpick comments (2)
packages/plugins/page/src/composable/usePage.ts (2)
117-143: Function namegenerateCssStringno longer reflects its behavior.The function now returns a CSS object, not a string. Consider renaming to
generateCssObjector similar for clarity.
118-118: Consider using proper types instead ofany.Using
anyforcssObjectandstyleObjloses type safety. A simple record type would improve maintainability.Suggested typing
- let cssObject: any = {} + let cssObject: Record<string, Record<string, string>> = {}- const styleObj: any = {} + const styleObj: Record<string, string> = {}Also applies to: 120-120
…into feat/schema-css-object
packages/settings/styles/src/components/classNamesContainer/index.vue
Outdated
Show resolved
Hide resolved
packages/settings/styles/src/components/classNamesContainer/index.vue
Outdated
Show resolved
Hide resolved
packages/settings/styles/src/components/classNamesContainer/index.vue
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/plugins/page/src/composable/usePage.ts (1)
33-57:⚠️ Potential issue | 🟡 MinorType inconsistency:
DEFAULT_PAGE.page_content.cssis a string butgenerateCssStringreturns an object.At line 39,
cssis initialized as an empty string''. WhengetDefaultPage()returnsDEFAULT_PAGEdirectly (line 150), consumers receivecss: ''(string). But when base styles exist (line 161), they receivecss: object. This inconsistency could cause runtime issues in downstream code expecting a consistent type.💡 Suggested fix for type consistency
const DEFAULT_PAGE = { app: '', name: '', route: '', page_content: { componentName: 'Page', - css: '', + css: {}, props: {},
🧹 Nitpick comments (2)
packages/utils/src/utils/index.ts (1)
462-462: Consider adding TypeScript type annotations.The function lacks type annotations which reduces type safety and IDE support. Since this is a utility function used across multiple packages, explicit typing would help catch integration issues early.
💡 Suggested improvement
-export const objectCssToString = (css) => { +export const objectCssToString = (css: string | Record<string, Record<string, string>> | null | undefined): string => {packages/plugins/page/src/composable/usePage.ts (1)
117-117: Function namegenerateCssStringis misleading since it now returns an object.Consider renaming to
generateCssObjectorgenerateCssStylesto better reflect the new return type and avoid confusion for future maintainers.💡 Suggested rename
-const generateCssString = (pageOptions: PageOptions, materialsOptions: MaterialsOptions) => { +const generateCssObject = (pageOptions: PageOptions, materialsOptions: MaterialsOptions) => {Also update the call site at line 161:
- css: generateCssString(pageOptions, materialsOptions) + css: generateCssObject(pageOptions, materialsOptions)
3 participants
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
页面schema中的样式类，css以字符串形式呈现，不利于展示和阅读，尤其是字符串很长的时候
Issue Number: N/A
What is the new behavior?
将页面schema中的css以对象形式呈现，同时处理回显和出码的逻辑
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.