Skip to content

refactor(style-editor): restructure style editor schema handling and …#35355

Open
KevinDavilaDotCMS wants to merge 4 commits intomainfrom
35270-unify-style-editor-schema-definition-use-content-type-tab-as-single-source-of-truth-for-headless-and-traditional-pages-frontend
Open

refactor(style-editor): restructure style editor schema handling and …#35355
KevinDavilaDotCMS wants to merge 4 commits intomainfrom
35270-unify-style-editor-schema-definition-use-content-type-tab-as-single-source-of-truth-for-headless-and-traditional-pages-frontend

Conversation

@KevinDavilaDotCMS
Copy link
Copy Markdown
Member

@KevinDavilaDotCMS KevinDavilaDotCMS commented Apr 16, 2026

…remove unused hooks

  • Introduced for normalizing style editor forms.
  • Updated and to register style editor schemas.
  • Removed hook and associated tests to streamline the codebase.
  • Added new types for style editor fields and schemas in .
  • Updated various components to utilize the new schema handling, improving maintainability and clarity.

This refactor enhances the style editor's integration and prepares for future feature expansions.

2026-04-16.14-21-37.mov

This PR fixes: #35270

…remove unused hooks

- Introduced  for normalizing style editor forms.
- Updated  and  to register style editor schemas.
- Removed  hook and associated tests to streamline the codebase.
- Added new types for style editor fields and schemas in .
- Updated various components to utilize the new schema handling, improving maintainability and clarity.

This refactor enhances the style editor's integration and prepares for future feature expansions.
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 16, 2026

Claude finished @dario-daza's task in 2m 32s —— View job


PR Review: Style Editor Schema Refactor

  • Gather context and read changed files
  • Review core schema/type changes
  • Review SDK changes (page-api, uve, angular, react)
  • Review component/store updates
  • Review example app changes
  • Post final review

Issues Found

1. console.log left in production codepage-api.ts:474

console.log('styleEditorSchemas', styleEditorSchemas);

This fires on every page load. Fix this →


2. Massive hardcoded mock schema ships in production SDKpage-api.ts:29–334

fetchStyleEditorSchemas always returns ~300 lines of hardcoded Activity schema data — it never calls a real API. Every page response from client.page.get() will include this fake schema payload in production. The TODO comment acknowledges the real endpoint doesn't exist yet, but shipping a mock as the implementation is the wrong call:

// TODO: Replace mock with real endpoint call like:
// GET /api/v1/style-editor/schemas?pageUrl=<url>
return Promise.resolve([{ contentType: 'Activity', sections: [...] }]);

This should either: (a) return [] until the endpoint is ready, or (b) be gated behind the feature flag that already exists.


3. Mock data contains third-party image URLspage-api.ts:125–144

The hardcoded mock references i.ibb.co (imgBB) URLs:

imageURL: 'https://i.ibb.co/cXv3tfYd/Screenshot-2025-12-23-at-11-58-32-AM.png'

Any app using this SDK will make cross-origin requests to imgBB on every page load. This is a privacy/reliability concern even as a temporary mock.


4. Feature flag override hardcoded in withFlagswithFlags.ts:35

// TODO: Remove this, only harcoded until the PR that fix is merged
flags[FeaturedFlags.FEATURE_FLAG_UVE_STYLE_EDITOR] = true;

This unconditionally enables the style editor regardless of what the backend returns. The feature flag cannot be disabled from the backend. This TODO was supposed to be temporary — what's the tracking issue for removal?


5. JSDoc vs. implementation mismatch in normalizeSectionuve/internal.ts:107–153

The JSDoc example shows fields as a 2D array (StyleEditorFieldSchema[][]) — suggesting a multi-column layout structure — but the actual implementation and the StyleEditorSectionSchema type both use a flat StyleEditorFieldSchema[]. The documentation actively misleads anyone reading the code.


6. Suspicious as unknown as double-castsuve/internal.ts:75–76

config.inputType = field.inputType as unknown as StyleEditorFieldInputType;
config.placeholder = field.placeholder as unknown as string;

These casts indicate real type incompatibilities being papered over. If field.inputType is already StyleEditorFieldInputType, no cast is needed. If it isn't, the types need to be fixed — not cast away.


7. defineStyleEditorSchema is an empty wrapperuve/internal.ts:237–239

export function defineStyleEditorSchema(form: StyleEditorForm): StyleEditorFormSchema {
    return normalizeForm(form);
}

This adds no validation, no unique behavior, nothing. Both defineStyleEditorSchema and normalizeForm are exported from @dotcms/uve/internal, making the API surface ambiguous. Either add real validation logic here or remove the indirection.


8. normalizeForm has zero test coverage

The PR deleted 294 lines of tests and replaced them with 12 lines that only test the styleEditorField builder helpers. The core transformation logic — normalizeField, normalizeSection, and normalizeForm — has no tests at all. This is the most critical path in this PR.


9. Checkbox keyvalue conversion is an undocumented semantic changeuve/internal.ts:91–98

Developers define checkbox options with a key property:

{ label: 'Bold', key: 'bold' }

But the normalization silently converts this to value for UVE:

config.options = field.options.map((opt) => ({ label: opt.label, value: opt.key }));

Meanwhile, the mock data in page-api.ts already uses value in checkboxGroup options, not key. When UVE returns saved checkbox state, will it use value identifiers that match what the developer's key was? This mapping needs to be documented and round-trip tested.


10. styleEditorSchemas spread guard is always truthypage-api.ts:518

...(styleEditorSchemas && { styleEditorSchemas })

fetchStyleEditorSchemas always resolves to a non-empty array (never null, never undefined). The guard never fires. This is dead code that creates a false sense of safety.


Summary

The architecture direction is sound — moving schemas to a server-driven single source of truth is the right call. But this PR shouldn't merge until the mock implementation in page-api.ts is replaced with either an empty return or proper feature-flag gating, the debug console.log is removed, and normalizeForm gets test coverage. Items 4–9 are lower priority but should be tracked.

…ernal module

- Updated imports across various components to utilize the new internal module for style editor types and schemas.
- Removed obsolete imports from the  package, streamlining the codebase.
- Introduced a new internal module for style editor types, enhancing organization and maintainability.
- Adjusted related services and components to align with the new structure, ensuring consistent usage of style editor schemas.

This refactor improves clarity and prepares the codebase for future enhancements in the style editor functionality.
@dario-daza dario-daza marked this pull request as ready for review April 20, 2026 16:10
…se-content-type-tab-as-single-source-of-truth-for-headless-and-traditional-pages-frontend
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Frontend PR changes Angular/TypeScript frontend code Area : SDK PR changes SDK libraries

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Unify Style Editor schema definition: use content type tab as single source of truth for headless and traditional pages

2 participants