viliusle / miniPaint

online image editor
http://viliusle.github.io/miniPaint/
Other
2.74k stars 634 forks source link

Implement smarter undo/redo system #218

Closed Giwayume closed 3 years ago

Giwayume commented 3 years ago

206

This implements a context-aware undo system that only saves the bare minimum data necessary to undo/redo any action performed in the application.

Image edits are saved to indexedDB. If that's not available (very unlikely), resorts to storing in memory.

This removes window.State.save(). Instead, a list of all history-bound actions that can be performed in miniPaint is located at src/js/actions. Each action implements a do() and undo() function, and stores all data necessary to switch between these states. The State object now has a do_action() method to run actions and add them to the history stack.

Other things of note:

API example, updates the position of the current layer.

import app from './js/app.js';

app.State.do_action(
    new app.Actions.Update_layer_action(config.layer.id, {
        x: 25,
        y: 100
    })
)
viliusle commented 3 years ago

Few issues:

app.State.do_action(
    new app.Actions.Bundle_action('translate_layer', 'Translate Layer', [
        new app.Actions.Update_layer_action(config.layer.id, {
            x: parseInt(params.x),
            y: parseInt(params.y)
        })
    ])
);
Giwayume commented 3 years ago

after refresh, opening any image, gives "Error: can not find layer with id:1" error. some issues with layers IDs: draw rectangle in middle, add image, drag image to center, select rectangle. Rectangle will auto jump to image location.

Will look

Why Bundle_action, why double "new app" init?. Can this be simplified from usage side?

The purpose of Bundle_action is to group a bunch of actions together into a single undo or redo. Since you're grouping, you must also provide a new ID and description for the group. For example some tools manipulate the image data then resize the canvas, those a 2 actions that should be undone all at once with one press of Ctrl + Z.

In the example you gave from the code I was using the bundle solely to rename the action, not to group. This is so in the future we can have an undo history list with properly labeled steps if desired.

image

Can you write some documentation for anybody new trying to write new module not knowing what to do about these states?,I would add it to states folder and name it like _README.txt.

I was thinking of opening a PR to update the wiki. You think that's a better idea or do you still want a readme.txt?

Giwayume commented 3 years ago

after refresh, opening any image, gives "Error: can not find layer with id:1" error.

This is now fixed. I broke it very recently when adding the exif data calculation to the history.

some issues with layers IDs: draw rectangle in middle, add image, drag image to center, select rectangle. Rectangle will auto jump to image location.

This wasn't a layer ID issue, the select tool calculated the mousedown position before the auto_select_object function runs, effectively calculating the position from the previously selected layer.

Let me know if you find any other bugs.

viliusle commented 3 years ago

readme.txt can contain basic info and link to wiki, or just a link, but wiki is good idea. Lets use it for main documentation.

Giwayume commented 3 years ago

I can't figure out how to open a PR to the wiki so I added the readme locally. I'll let you manage putting it in the wiki.

viliusle commented 3 years ago

Wiki page created: https://github.com/viliusle/miniPaint/wiki/Undo-Redo-system

viliusle commented 3 years ago

Another issue: opening image from "search images" on left sidebar works first time, seconds, time, but not anymore.

Giwayume commented 3 years ago

It seems to quit after a while, but I have to do like 7-8 searches before it breaks.

Giwayume commented 3 years ago

219