feat: toolbar to headless api (state and execute)#2757
feat: toolbar to headless api (state and execute)#2757artem-harbour wants to merge 1 commit intomainfrom
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
artem-harbour
force-pushed
the
artem/headless-toolbar-migration-v1
branch
from
3687083 to
bad1282
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: bad1282efc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
packages/super-editor/src/editors/v1/components/toolbar/super-toolbar.js
Show resolved
Hide resolved
packages/super-editor/src/editors/v1/components/toolbar/super-toolbar.js
Show resolved
Hide resolved
artem-harbour
force-pushed
the
artem/headless-toolbar-migration-v1
branch
from
bad1282 to
dfeadfc
Compare
|
@harbournick @caio-pizzol - please review this. |
artem-harbour
force-pushed
the
artem/headless-toolbar-migration-v1
branch
from
dfeadfc to
82c59ac
Compare
artem-harbour
force-pushed
the
artem/headless-toolbar-migration-v1
branch
from
82c59ac to
51dad56
Compare
caio-pizzol
left a comment
caio-pizzol
left a comment
There was a problem hiding this comment.
@artem-harbour clean migration, the routing is much easier to follow now. left a few inline comments.
two things to fix: toggling formatting on empty selections loses the toggle when you click away (sticky marks), and image upload errors no longer reach onException. also flagged three lists in constants.js that could get out of sync — easy to derive them from the maps.
test-wise: no coverage for destroy/cleanup and no test for hiding tracked-change buttons based on role. not blocking, good follow-ups.
| if (handledByHeadless) { | ||
| this.updateToolbarState(); | ||
| if (shouldRestoreFocus) this.#scheduleRestoreEditorFocus(); | ||
| return; |
There was a problem hiding this comment.
when you toggle bold/italic/underline on an empty selection via toolbar, the new headless path returns before saving the sticky mark state. so if the user clicks somewhere else after toggling, the formatting is lost. adding the sync call before the return should fix it:
| return; | |
| if (isMarkToggle) this.#syncStickyMarksFromState(); | |
| return; |
| @@ -849,20 +520,157 @@ export class SuperToolbar extends EventEmitter { | |||
| } | |||
There was a problem hiding this comment.
the old image upload code fired an 'exception' event when something went wrong, so consumers could handle it via onException. the new headless version just logs to console — no event reaches consumers anymore. worth wiring the error back up?
| { label: '96', key: '96pt', props: { 'data-item': 'btn-fontSize-option' } }, | ||
| ]; | ||
|
|
||
| export const HEADLESS_TOOLBAR_COMMANDS = [ |
There was a problem hiding this comment.
TABLE_ACTION_COMMAND_IDS, HEADLESS_TOOLBAR_COMMANDS, and HEADLESS_EXECUTE_ITEMS are all just copies of the data in the two maps above. if someone adds a command to one map but forgets to update these lists, things break silently. can we derive them from the maps instead?
artem-harbour
force-pushed
the
artem/headless-toolbar-migration-v1
branch
from
51dad56 to
b1cc8d2
Compare
artem-harbour
force-pushed
the
artem/headless-toolbar-migration-v1
branch
from
b1cc8d2 to
600a652
Compare
|
@harbournick @caio-pizzol - please review and merge. |
3 participants
Linear: SD-2348.
Migrated toolbar to headless API (state and execute path).
Notes:
LinkInput.vue).getFontFamilyFallbackValue,isFontSizeMixedState) rather than headless api for now.