Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add useStateWithHistory hook and use to show a block editor with undo/redo #54377
Add useStateWithHistory hook and use to show a block editor with undo/redo #54377
Changes from 1 commit
ee9baac
e4c17bc
e364fdf
734899f
85cf805
72c4404
cd69c61
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / Playwright - 1
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / Playwright - 4
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / Puppeteer - 3
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / Playwright - 3
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / Puppeteer - 1
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / Playwright - 2
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / PHP 7.1 multisite on ubuntu-latest
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / Puppeteer - 2
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / PHP 7.2 multisite on ubuntu-latest
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / PHP 7.3 on ubuntu-latest
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / PHP 7.1 on ubuntu-latest
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / PHP 7.2 on ubuntu-latest
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / Check
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / PHP 8.1 multisite on ubuntu-latest
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / PHP 8.0 multisite on ubuntu-latest
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / PHP 7.4 on ubuntu-latest
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / PHP 7.4 multisite on ubuntu-latest
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / PHP 8.0 on ubuntu-latest
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / PHP 7.4 (WP 6.2.2) on ubuntu-latest
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / PHP 7.0 (WP 6.2.2) on ubuntu-latest
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / PHP 8.1 on ubuntu-latest
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / PHP 8.2 (WP 6.2.2) on ubuntu-latest
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / PHP 7.3 multisite on ubuntu-latest
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / PHP 7.0 multisite on ubuntu-latest
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / PHP 7.0 on ubuntu-latest
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / PHP 8.2 on ubuntu-latest
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / Run performance tests
Check failure on line 4 in packages/compose/src/hooks/use-state-with-history/index.ts
GitHub Actions / Build Release Artifact
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't the internal state be equal to that of the under manager?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I thought the state was exposed, but I guess not
gutenberg/packages/undo-manager/src/index.js
Line 75 in 40c9f17
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why no give access to the state though, if it's a low level thing? Then you don't need an intermediate state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure, it feels like an internal representation of a history. I think regardless of whether we expose it or not, we need an intermediate state, in some occasions we don't record undo/redo steps. (undoIgnore option)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we have some global keyboard shortcuts that conflict playground's shortcuts (try typing in the editor). Feels like we may want to call "preventDefault" when typing random characters in the editor but it's out of scope here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding this to the code example doesn't feel right. Should maybe move to a general playground template
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's a real bug in our components, I'm not sure where though because when you type within regular inputs the events are ignored by playwright but when you type in the block editor, the events reach the playground. So I'd rather not hide it for now.