Skip to content
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.

Sign up for GitHub

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

Jump to bottom

undo redo history #111

Merged
merged 19 commits into from
Oct 4, 2023
Merged

undo redo history #111

merged 19 commits into from
Oct 4, 2023

Conversation

Yuddomack
Copy link
Contributor

hi! I tried working on it.
but I did this task without fully understand, so need to some your feedback

thank you!

@vercel
Copy link

vercel bot commented Sep 26, 2023

@Yuddomack is attempting to deploy a commit to the Measured Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Sep 26, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
puck-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 4, 2023 7:56am

chrisvxd
chrisvxd requested changes Sep 26, 2023
View reviewed changes
Copy link
Member

@chrisvxd chrisvxd left a comment

Choose a reason for hiding this comment

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

Thanks for the big effort again @Yuddomack!

I've left a suggestion for how to simplify the history recording using a diffing library.

packages/core/jest.config.ts Show resolved Hide resolved
packages/core/components/Puck/index.tsx Outdated Show resolved Hide resolved
packages/core/lib/reducer.ts Outdated Show resolved Hide resolved
packages/core/lib/use-action-history.ts Show resolved Hide resolved
packages/core/lib/use-puck-data.ts Outdated Show resolved Hide resolved
@chrisvxd chrisvxd marked this pull request as ready for review October 2, 2023 08:43
chrisvxd
chrisvxd requested changes Oct 2, 2023
View reviewed changes
Copy link
Member

@chrisvxd chrisvxd left a comment

Choose a reason for hiding this comment

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

This is really excellent work! Thank you 🙏

Very happy with this approach. Most of my feedback is on phrasing in the unit tests.

packages/core/lib/use-puck-history.ts Show resolved Hide resolved
packages/core/lib/use-puck-history.ts Outdated Show resolved Hide resolved
packages/core/package.json Show resolved Hide resolved
packages/core/lib/__tests__/use-action-history.spec.tsx Outdated Show resolved Hide resolved
packages/core/lib/__tests__/use-action-history.spec.tsx Outdated Show resolved Hide resolved
packages/core/lib/__tests__/use-action-history.spec.tsx Outdated Show resolved Hide resolved
packages/core/lib/__tests__/use-action-history.spec.tsx Outdated Show resolved Hide resolved
packages/core/lib/__tests__/use-action-history.spec.tsx Outdated Show resolved Hide resolved
packages/core/lib/use-puck-history.ts Show resolved Hide resolved
packages/core/lib/__tests__/use-action-history.spec.tsx Show resolved Hide resolved
Yuddomack and others added 15 commits October 3, 2023 20:24
@Yuddomack
chore: test environment setting
282e3ef
@Yuddomack
feat: init canForward, canRewind
57a55c3
@Yuddomack
feat: rewind, forward execute functions that return to a specific poi…
c6d2098 
…nt in time
@Yuddomack
feat: overwrite when record again after rewind
4ff0e1b
@Yuddomack
feat: defines the opposite of each action
eb722f9
@Yuddomack
feat: apply shortcuts
bea841a
@Yuddomack
fix: from custom color to puck color
e6ae941
@Yuddomack
feat: add 'deep-diff', 'event-emitter' dependencies
bb354f5
@Yuddomack
fix: changed the history mechanism from action-based to diff-based to…
27b7a36 
… minimize side effects on reducer
@Yuddomack
fix: revert the separated hook and apply only history related matters
c7f14aa
@Yuddomack
fix: debounced recording to save space
ec76cf9
@Yuddomack
fix: remove unused code
f20f794
@Yuddomack @chrisvxd
fix: Update packages/core/lib/__tests__/use-action-history.spec.tsx
4a271a3 
Co-authored-by: Chris Villa <[email protected]>
@Yuddomack
fix: add test process in ci.yml
ce2b29a
@Yuddomack
fix: The meta key is the same as cmd on macOS and os key on Windows i…
c4148b4 
…n react-hotkeys-hook
@Yuddomack
Copy link
Contributor Author

Hi! @chrisvxd Your request has arrived!

chrisvxd
chrisvxd requested changes Oct 3, 2023
View reviewed changes
Copy link
Member

@chrisvxd chrisvxd left a comment

Choose a reason for hiding this comment

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

Wonderful! A couple more tiny comments before approval.

We're planning on doing v0.8.0@latest release soon, so will hold off merging this until after that goes in to give it a chance to be tested in canary.

packages/core/package.json Show resolved Hide resolved
packages/core/lib/use-puck-history.ts Show resolved Hide resolved
packages/core/package.json Outdated
"react-beautiful-dnd": "^13.1.1",
"react-feather": "^2.0.10",
Copy link
Member

Choose a reason for hiding this comment

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

This looks like a merge conflict - react-feather should have been removed in favour of the devDependency (see #121)

packages/core/lib/use-puck-history.ts Show resolved Hide resolved
@chrisvxd chrisvxd mentioned this pull request Oct 3, 2023
Merged
@Yuddomack
Copy link
Contributor Author

Hi! @chrisvxd
Additional requests are also prepared! (maybe? 😆)

chrisvxd
chrisvxd approved these changes Oct 4, 2023
View reviewed changes
Copy link
Member

@chrisvxd chrisvxd left a comment

Choose a reason for hiding this comment

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

🚀 Epic work @Yuddomack!

@chrisvxd chrisvxd merged commit 222697e into measuredco:main Oct 4, 2023
2 checks passed
@chrisvxd chrisvxd linked an issue Oct 4, 2023 that may be closed by this pull request
Undo/redo history #23
Closed
@chrisvxd
Copy link
Member

chrisvxd commented Oct 4, 2023

@Yuddomack available in 0.9.0-canary.222697e!

@chrisvxd chrisvxd mentioned this pull request Oct 4, 2023
Closed
@chrisvxd
Copy link
Member

chrisvxd commented Oct 9, 2023

@Yuddomack would love to have you in the Discord, if you're interested: https://discord.com/invite/D9e4E3MQVZ

@Yuddomack
Copy link
Contributor Author

@chrisvxd cool! I joined it

@Yuddomack Yuddomack deleted the undo-redo branch October 10, 2023 00:36
@chrisvxd chrisvxd mentioned this pull request Oct 17, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chrisvxd chrisvxd chrisvxd approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Undo/redo history
2 participants
@Yuddomack @chrisvxd