Skip to content

fix(ui): align editor appearance across all five bindings - #359

Merged
ChristopherVR merged 1 commit into
ChristopherVR:mainfrom
yunfeizhu:fix/editor-chrome-parity
Sep 30, 2026
Merged

ChristopherVR merged 1 commit into
ChristopherVR:mainfrom
yunfeizhu:fix/editor-chrome-parity

Conversation

@yunfeizhu

@yunfeizhu yunfeizhu commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Unify the uploaded-deck editor appearance across React, Vue, Angular, Svelte and Vanilla, using React as the reference. This continues #342 after the upload landing changes in #354.

  • Centralize editor metrics, scoped CSS and Home ribbon artwork in pptx-viewer-shared.
  • Align split New Slide buttons, icon spacing, caret placement, tab underline, disabled backgrounds, inspector cards, notes and viewport fit.
  • Reuse pptx-ui-select for both font fields in all five bindings, with 120px family and 64px size widths and the agreed 12px/18px typography.
  • Align thumbnail selection, 3px markers, number colors and 8px frame spacing.
  • Dock the Vanilla theme editor at Design > Edit Theme in the React position.

Review repairs

  • Both React font fields now use the existing native custom-element adapter. Regression tests cover disabled/value resync, callback replacement and native listener cleanup under React 18 and 19.
  • Reduce shared size-field inner padding so 40.5 remains visible within the fixed 64px field. The five-binding first-screen checks assert no clipping or adjacent-control movement.
  • Remove font-baseline-dependent extra height from Home control wrappers; check the same 28px rows with system and Arial font metrics.
  • Correct scaled row-boundary hit detection in Vue, Svelte and Vanilla so table-cell selection is not mistaken for a resize gesture. React and Angular use explicit handles; all five table accessibility flows are covered.
  • Read each generated fixture's actual first-slide layout for master/template workflows, preserving the saved-layout and artwork assertions. Core sources and these fixture generators are unchanged from the previous base.
  • Await thumbnail transitions before asserting final colors, address the actual shadow comboboxes, and inventory each custom form control once while still traversing migrated command roots.
  • Record the selected canvas element's native animation-start event before clicking Preview. The 500ms effect had already finished before a busy runner sampled its style; recording actual playback preserves the in-place animation assertion through cleanup.
  • Integrate upstream main a1549a09, retaining its Slide Show, Help, Record and Subtitle Settings migrations. Update the affected SSR and Angular translation test harnesses, React Help/boolean-attribute assertions, and write-back mock isolation. Wait for controlled checkbox state to be committed before checking the saved deck, and measure visible ribbon groups.
  • Complete all four locale dictionaries for the three new subtitle-setting strings and regenerate the customization reference for accessibility.subtitleLanguage. Existing dictionary and documentation completeness assertions are retained.
  • Read Record command labels and group captions from their rendered shadow DOM in the React and Vue localization tests. Retain the complete localized/English label comparisons, disabled states and recording callback assertions.

Verification after the latest upstream integration

  • Root bun run build and bun run typecheck; React 18 type checking.
  • Complete shared coverage suite: 10,901 passed and 3 skipped. Complete locales coverage suite: 35 passed.
  • Complete React 19 and React 18 coverage suites: 7,527 passed each. Complete Vue coverage suite: 3,158 passed. React 18 type checking passed after the coverage run.
  • Rebuilt locales, checked their types, and validated all 20 packed locale subpaths across the five bindings, including ESM, CommonJS, NodeNext and Bundler declarations.
  • Focused font, toolbar, Record and Subtitle Settings regressions: React 19 and React 18 175 passed each; Angular 24; Vue 13; Svelte 12; Vanilla 42.
  • Latest-base browser verification: all 60 first-screen, ribbon-inventory, animation-preview, Record and Subtitle Settings cases passed across the five bindings without retries or skips.

Earlier verification of the PR changes

  • Root lint, format checking, E2E neutrality, commit hooks and git diff --check.
  • Packed core and five isolated binding consumers; all five published-type and external-Yjs checks.
  • CI script tests in a shallow checkout: 123 passed, 1 skipped.
  • All eight complete CI coverage suites on the preceding upstream base: 37,268 passed and 3 skipped. This includes both React 18 and 19 and all five bindings.
  • Browser rerun against all five existing services: all 285 cases across 13 affected specs passed without failures, retries or skips. This covers first-screen parity, font changes, ribbon inventory, master/template edits, table selection, Help/Slide Show migration and persisted settings.
  • Animation Preview playback and enabled-state checks: 10 additional cases passed across all five bindings without retries.

Local checks use macOS and Node 24.19; hosted CI uses Ubuntu and Node 22. The React 18 local coverage run is serial. Browser checks use the existing contributor services with webServer: [].

Commit organization

All five bindings, shared UI, related tests and design notes are in one Conventional Commit, authored and committed as Euan Zhu.

@github-actions github-actions Bot added documentation Improvements or additions to documentation react React binding (pptx-react-viewer) vue Vue binding (pptx-vue-viewer) angular Angular binding (pptx-angular-viewer) svelte Svelte binding (pptx-svelte-viewer) vanilla Vanilla binding (pptx-vanilla-viewer) shared Framework-agnostic viewer logic (pptx-viewer-shared) e2e Playwright end-to-end suite labels Sep 30, 2026

@ChristopherVR ChristopherVR left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The shared chrome/style ownership and five-binding coverage fit #342, but this head is not ready to merge. I reproduced the React 18 regression in an isolated checkout at a94934c: bunx vitest run --config vitest.react18.config.ts src/viewer/components/toolbar/FontPickers.test.tsx src/viewer/components/toolbar/RibbonCommandEffects.test.tsx gives 8 failed and 4 passed. Please use a native custom-element adapter for both font selectors and rerun the same tests under React 18 and React 19.

CI also fails the PR's new Vanilla decimal-size test at e2e/editor-first-screen-parity.spec.ts:173: the 40.5 value is clipped (decimal.clipped is true), repeated on both retries in job 109801531213. Please preserve the promised 64px field while allowing the decimal text to fit, and rerun that spec across all five bindings. Other failing e2e jobs include master/template workflows and ribbon customization; please triage them against main before requesting merge.

I am keeping the next Slide Show migration on current main for now. Its direct overlap with this PR is limited to the shared render barrel and React/Vanilla tests; Home chrome and the theme-editor move should stay owned by this PR. #364 records the theme-editor verification and will be reconciled after this lands. The migration tracker is #342, with #360-#365 as scoped work items.

@yunfeizhu

Copy link
Copy Markdown
Contributor Author

Thanks, @ChristopherVR. I've updated both font selectors to use the native adapter and checked them under React 18 and 19. The decimal-size clipping is also fixed while keeping the 64px field. I've checked the master/template and ribbon issues against main and completed the regression checks across all five bindings.

I've integrated your latest Slide Show and Help changes from main, and kept Home chrome and the Vanilla theme-editor move in this PR as discussed. We can reconcile #364 after this lands.

@ChristopherVR

Copy link
Copy Markdown
Owner

Rechecked revision 6df72ea. The previous React 18 FontPickers blocker is addressed: both fields now use WebSelect's native property/listener lifecycle. The isolated React 18 FontPickers and Toolbar run passes all 148 tests, including fractional sizes, native changes and disabled transitions. Build, lint and typecheck CI have passed as well.

I am keeping this open while the new CI failure is resolved. Angular shard 6/8 fails ribbon-animation-preview-parity.spec.ts:51 on all three attempts with react: Preview did not set a real CSS animation on the selected element. Despite the job name, this is a cross-binding test reporting the React preview path. Please fix that regression, or demonstrate it on the unchanged base and track the independent failure before merging.

The #363 Record batch and #365 subtitle settings continue on main without duplicating this PR's Home/editor-chrome work.

@yunfeizhu
yunfeizhu force-pushed the fix/editor-chrome-parity branch 3 times, most recently from 834bf9a to b8adb5e Compare September 30, 2026 11:55
Share editor metrics, Home icons, controls and thumbnail spacing across
React, Vue, Angular, Svelte and Vanilla. Use the shared font selectors
with native React 18 lifecycle wiring and keep decimal sizes visible.

Dock the Vanilla theme editor in the React position and cover visual
parity, scaled table selection and current fixture layouts in tests.
@yunfeizhu
yunfeizhu force-pushed the fix/editor-chrome-parity branch from b8adb5e to 39d0943 Compare September 30, 2026 12:01
@github-actions github-actions Bot added the i18n Translations and locale dictionaries label Sep 30, 2026
@yunfeizhu

Copy link
Copy Markdown
Contributor Author

Thanks, @ChristopherVR. The remaining issues are addressed in 39d0943ee. The Preview test was observing the 500ms effect after cleanup; it now listens for the selected canvas element's native animationstart event before clicking Preview. It still verifies that the effect plays in place and the editor remains open. I've checked this across all five bindings and rerun the full React 18 and 19 suites.

I've also resolved the conflicts with current main, completed the subtitle translations and generated customization reference, and updated the Record localization tests for the Web Components. Everything remains in one commit, and the font-selector thread is marked resolved. Ready for another review when you have time.

@ChristopherVR
ChristopherVR merged commit 178a1d0 into ChristopherVR:main Sep 30, 2026
60 of 63 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

angular Angular binding (pptx-angular-viewer) documentation Improvements or additions to documentation e2e Playwright end-to-end suite i18n Translations and locale dictionaries react React binding (pptx-react-viewer) shared Framework-agnostic viewer logic (pptx-viewer-shared) svelte Svelte binding (pptx-svelte-viewer) ui-migration Shared Web Component migration tracked under #342 vanilla Vanilla binding (pptx-vanilla-viewer) vue Vue binding (pptx-vue-viewer)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants