feat(table): add configurable table width export mode#888
Conversation
🦋 Changeset detectedLatest commit: e3cd174 The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughThis PR introduces a new ChangesTable Width Export Mode Feature
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/core/src/config/interface.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. packages/editor/__tests__/create.test.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. packages/table-module/src/module/elem-to-html.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
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 |
cycleccc
force-pushed
the
chore/triage-issue-885
branch
from
e23c48d to
e3cd174
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e23c48d533
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| const menuConf = editor.getMenuConfig('insertTable') as { widthExportMode?: TableWidthExportMode } | ||
|
|
||
| return menuConf?.widthExportMode === 'explicit' ? 'explicit' : 'adaptive' |
There was a problem hiding this comment.
Treat missing/unknown widthExportMode as 'explicit' here; the current fallback returns 'adaptive' for any non-'explicit' value, including when editor.getMenuConfig('insertTable') returns {} or a malformed config. In those cases, tables with width: auto + columnWidths stop exporting pixel widths and unexpectedly emit width:auto, which breaks the commit’s backward-compatibility guarantee for default behavior.
Useful? React with 👍 / 👎.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/editor/__tests__/create.test.ts (1)
188-236: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winRound-trip coverage is still incomplete for this serialization change.
Please add explicit checks for
toHtml(parseHtml(html))and the explicit-mode round-trip path
alongside the currentsetHtml(getHtml())adaptive assertion.As per coding guidelines, "
**/*.test.{ts,tsx}: Test round-trip export/import ... across render/toHtml/parseHtml paths, and with setHtml(getHtml())/toHtml(parseHtml(html)) when modifying serialization or styles".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/editor/__tests__/create.test.ts` around lines 188 - 236, Add assertions that exercise the toHtml(parseHtml(html)) path and the explicit-mode round-trip in addition to the existing setHtml(getHtml()) adaptive checks: call the parser/render path (use parseHtml(html) and the toHtml helper or equivalent) and assert its output contains the same expected table style/colgroup strings, and for the explicit width case recreate the editor with MENU_CONF.insertTable.widthExportMode set to 'explicit' (or use customCreateEditor({ html, config: { MENU_CONF: { insertTable: { widthExportMode: 'explicit' } } } })) then assert both toHtml(parseHtml(html)) and setHtml(getHtml()) round-trips produce the expected 'style="width: 200px;table-layout: fixed;' and the colgroup HTML; keep the existing adaptive test unchanged but add these extra expectations for round-trip coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/config/interface.ts`:
- Around line 118-127: The new widthExportMode property was added to
IInsertTableConfig but the runtime and tests expect it on the menu config key
used by MENU_CONF.insertTable (IMenuConfig currently maps it to insertTableRow),
so remove widthExportMode from IInsertTableConfig and instead add/align it to
the menu config type that represents MENU_CONF.insertTable (update
IMenuConfig/ISingleMenuConfig mapping or the explicit insertTable type so that
IMenuConfig.insertTable includes widthExportMode); reference IInsertTableConfig,
IMenuConfig, ISingleMenuConfig, insertTableRow, and insertTable to locate and
adjust the types accordingly.
In `@packages/table-module/src/module/elem-to-html.ts`:
- Around line 22-30: The current getTableWidthExportMode function treats any
non-'explicit' value (including undefined) as 'adaptive'; change the logic so
missing/invalid config falls back to 'explicit' instead. In
getTableWidthExportMode, read menuConf?.widthExportMode and return 'adaptive'
only when it is explicitly 'adaptive' (e.g., menuConf?.widthExportMode ===
'adaptive' ? 'adaptive' : 'explicit'); keep checks for editor and getMenuConfig
as-is and update the return expression accordingly.
---
Outside diff comments:
In `@packages/editor/__tests__/create.test.ts`:
- Around line 188-236: Add assertions that exercise the toHtml(parseHtml(html))
path and the explicit-mode round-trip in addition to the existing
setHtml(getHtml()) adaptive checks: call the parser/render path (use
parseHtml(html) and the toHtml helper or equivalent) and assert its output
contains the same expected table style/colgroup strings, and for the explicit
width case recreate the editor with MENU_CONF.insertTable.widthExportMode set to
'explicit' (or use customCreateEditor({ html, config: { MENU_CONF: {
insertTable: { widthExportMode: 'explicit' } } } })) then assert both
toHtml(parseHtml(html)) and setHtml(getHtml()) round-trips produce the expected
'style="width: 200px;table-layout: fixed;' and the colgroup HTML; keep the
existing adaptive test unchanged but add these extra expectations for round-trip
coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7578b7d8-a29f-4af6-a5be-d51dcde54398
📒 Files selected for processing (5)
.changeset/calm-news-prove.mdpackages/core/src/config/interface.tspackages/editor/__tests__/create.test.tspackages/table-module/src/module/elem-to-html.tspackages/table-module/src/module/menu/index.ts
| interface IInsertTableConfig { | ||
| minWidth: number; | ||
| minRowHeight: number; | ||
| tableHeader: { | ||
| selected: boolean; | ||
| }; | ||
| tableFullWidth: { | ||
| selected: boolean; | ||
| } | ||
| }; | ||
| widthExportMode: TableWidthExportMode; |
There was a problem hiding this comment.
widthExportMode is typed on the wrong menu config contract key.
widthExportMode was added to IInsertTableConfig, but IMenuConfig currently binds that type to
insertTableRow, while runtime/test usage reads MENU_CONF.insertTable. This makes the new public
type contract inaccurate and hides the real config shape behind ISingleMenuConfig’s index signature.
Suggested contract alignment
interface IMenuConfig {
- insertTable: ISingleMenuConfig;
+ insertTable: IInsertTableConfig;
deleteTable: ISingleMenuConfig;
- insertTableRow: IInsertTableConfig;
+ insertTableRow: ISingleMenuConfig;
deleteTableRow: ISingleMenuConfig;
...
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/config/interface.ts` around lines 118 - 127, The new
widthExportMode property was added to IInsertTableConfig but the runtime and
tests expect it on the menu config key used by MENU_CONF.insertTable
(IMenuConfig currently maps it to insertTableRow), so remove widthExportMode
from IInsertTableConfig and instead add/align it to the menu config type that
represents MENU_CONF.insertTable (update IMenuConfig/ISingleMenuConfig mapping
or the explicit insertTable type so that IMenuConfig.insertTable includes
widthExportMode); reference IInsertTableConfig, IMenuConfig, ISingleMenuConfig,
insertTableRow, and insertTable to locate and adjust the types accordingly.
| function getTableWidthExportMode(editor?: IDomEditor): TableWidthExportMode { | ||
| if (!editor || typeof editor.getMenuConfig !== 'function') { | ||
| return 'explicit' | ||
| } | ||
|
|
||
| const menuConf = editor.getMenuConfig('insertTable') as { widthExportMode?: TableWidthExportMode } | ||
|
|
||
| return menuConf?.widthExportMode === 'explicit' ? 'explicit' : 'adaptive' | ||
| } |
There was a problem hiding this comment.
Default fallback is inverted to adaptive when config is missing/undefined.
This function currently defaults to adaptive for any non-explicit value. That breaks the stated
backward-compatible default. Missing/invalid config should resolve to explicit.
Suggested fix
- return menuConf?.widthExportMode === 'explicit' ? 'explicit' : 'adaptive'
+ return menuConf?.widthExportMode === 'adaptive' ? 'adaptive' : 'explicit'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/table-module/src/module/elem-to-html.ts` around lines 22 - 30, The
current getTableWidthExportMode function treats any non-'explicit' value
(including undefined) as 'adaptive'; change the logic so missing/invalid config
falls back to 'explicit' instead. In getTableWidthExportMode, read
menuConf?.widthExportMode and return 'adaptive' only when it is explicitly
'adaptive' (e.g., menuConf?.widthExportMode === 'adaptive' ? 'adaptive' :
'explicit'); keep checks for editor and getMenuConfig as-is and update the
return expression accordingly.
1 participant
Summary
This PR adds a configurable table width export strategy and keeps backward compatibility by default.
MENU_CONF.insertTable.widthExportModewith two modes:explicit(default, legacy-compatible)adaptive(preservewidth:autoon export)explicitto avoid breaking existing integrations.Closes #885
Why
Issue #885 reports that imported
width:auto; table-layout: fixedtables become fixed pixel width after export. Some users expect auto width to be preserved, while others rely on explicit pixel-width export. This PR supports both via config.Behavior
explicit): unchanged behavior.adaptive): when table model width isauto, export keepswidth:autoinstead of converting fromcolumnWidthsto px.Risk
explicit.adaptive(changed export contract by configuration).Rollback
MENU_CONF.insertTable.widthExportMode = 'explicit'.Validation
pnpm --filter @wangeditor-next/editor exec vitest run __tests__/create.test.ts -t "table width|explicit mode|adaptive mode|colgroup widths|by default" --reporter=dotpnpm --filter @wangeditor-next/table-module exec vitest run __tests__/elem-to-html.test.ts --reporter=dotpnpm --filter @wangeditor-next/table-module exec vitest run __tests__/parse-html.test.ts -t "colgroup widths|auto height|class mode attrs|width 100%" --reporter=dotpnpm exec eslint packages/core/src/config/interface.ts packages/editor/__tests__/create.test.ts packages/table-module/src/module/elem-to-html.ts packages/table-module/src/module/menu/index.tspnpm turbo build --filter=@wangeditor-next/table-module --filter=@wangeditor-next/editorSummary by CodeRabbit
adaptive(preserves flexiblewidth:autostyling) andexplicit(applies computed fixed widths). Default isexplicitfor backward compatibility.