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.

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 Undo/Redo #149

Merged
merged 10 commits into from
Jul 22, 2024
Merged

Add Undo/Redo #149

merged 10 commits into from
Jul 22, 2024

Conversation

mnsrv
Copy link
Contributor

@mnsrv mnsrv commented Jul 20, 2024

Resolves #148

  • fromUndoRedo is needed to prevent pushing prev color to undo, after we updated current in undo/redo.
  • undos stores current as last item, so it never should be empty.
  • $prev is needed for storing previous color. I tried to use oldValue from subscribe, but because of debounce, it can be the same as value.
  • $prev initialised with 0 color to avoid checking for null. I set correct first value in subscribe, when oldValue is not passed
  • on cmd + Y in macOS Safari usually opens history, is it okay that now I prevent to open it?

@mnsrv mnsrv changed the title Add Undo/Redo #148 Add Undo/Redo Jul 20, 2024
view/base/index.ts Outdated Show resolved Hide resolved
stores/history.ts Outdated Show resolved Hide resolved
stores/history.ts Outdated Show resolved Hide resolved
import { debounce } from '../lib/time.js'
import { current, type LchValue } from './current.js'

let $undos = atom<LchValue[]>([])
Copy link
Member

Choose a reason for hiding this comment

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

Do we have $ in store name in other stores?

If not it is better to keep origin name system.

We can switch to $ in separated PR. I like this idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done. I understand it and it was that before. but with prev, I few times was confused is it a variable or a store. and in documentation I saw this $ sign. then I left with undos store and $prev store

Copy link
Member

Choose a reason for hiding this comment

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

If you have time, let's migrate to $ when we will finish with this PR

view/base/index.ts Outdated Show resolved Hide resolved
lib/hotkeys.ts Outdated Show resolved Hide resolved
lib/hotkeys.ts Outdated Show resolved Hide resolved
lib/hotkeys.ts Outdated Show resolved Hide resolved
lib/hotkeys.ts Outdated Show resolved Hide resolved
view/base/index.ts Outdated Show resolved Hide resolved
@ai ai merged commit 9b21c9b into evilmartians:main Jul 22, 2024
2 checks passed
lib/hotkeys.ts Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Undo/Redo
2 participants