Fix history by tracking initial empty state and removal of segments

hundredrabbits / Dotgrid

Minimalist Vector Tool

https://hundredrabbits.itch.io/dotgrid

Other

1.04k stars 75 forks source link

Fix history by tracking initial empty state and removal of segments #71

Closed fkettelhoit closed 4 years ago

fkettelhoit commented 4 years ago

Fixes 2 small issues:

I found it a bit unexpected that translating a segment is part of the undo/redo history, while removing a segment is not.
The first shape that is initially drawn cannot be undone since there is no 'empty state' to revert back to.

neauoire commented 4 years ago

Sorry for the delay, I like this PR a lot, but I'd like this fix to not change the history.js file since it is shared with other applications. Could you implement this fix by not changing the history file?

fkettelhoit commented 4 years ago

Sure, I'll take a look in the next few days and update the PR once I have made the changes.

Have you seen the other 2 open PRs? I'm just asking since I need to resolve the conflicts introduced by the recent commits and wanted to make sure that these features are or any interest before doing any more work on the branches. (And if you're actively working on Dotgrid right now, I might hold off until master stabilizes a bit.)

neauoire commented 4 years ago

Yes, sorry. I've had a lot of changes on the backburner, and when I saw your PR I was like, shit, maybe I should start pushing these changes first.

Your PRs change the default behaviour which will break the export for most of our files, so I didn't merge them yet since we use it for a bunch of automated tasks. I've started to make some changes to your PR to give the cropping as an option. Let's do the history first, and look at the crop export after :)

fkettelhoit commented 4 years ago

No problem, I already expected the viewBox PR to be a bit problematic. I'll hold off on that for the time being and do the changes for the history PR here once I find the time.

(And btw, I would totally understand if the whole viewBox / mask layer features are something that you don't want for Dotgrid. I hacked it together since I needed it myself and figured it wouldn't hurt to open PRs, but the whole thing is a bit of an odd feature compared to the rest of Dotgrid.)

Anyway, thanks for considering the features, I'll update this PR as soon as I can.

neauoire commented 4 years ago

I had some time tonight so I fixed this, turns out I only needed to push an initial state on client.start()