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

Feature: History/Undo/Redo #54

Merged
merged 3 commits into from
May 26, 2023
Merged

Conversation

wainwrightmark
Copy link
Contributor

@wainwrightmark wainwrightmark commented May 20, 2023

#53 (comment)

This change adds a yewdux-utils crate with a listener that tracks the history of a state and enables undo/redo.
It works very nicely - try running new history example.

There's a couple of issues with it that I would like some feedback on.

Firstly, it should be able to initialize it by just injecting the listener using the attribute macro like this:

#[derive(Debug, Default, Clone, PartialEq, Eq)]
#[store(listener(HistoryListener<State>)]
struct State {
    count: u32,
}

but the person who implemented that attribute macro (it was me!) decided to use PathList which apparently doesn't support generic types. I've asked on the Darling repo but I'm pretty sure there's no way to make that work.
So you have to initialize it the old way

impl Store for State {
    fn new() -> Self {
        init_listener(HistoryListener::<State>::default());
        Self::default()
    }

    fn should_notify(&self, old: &Self) -> bool {
        self != old
    }
}

The other thing is that the undo tracking currently tracks a potentially infinite number of states which could lead to running out of memory quite quickly if you don't clear it periodically. I think it might be better to let you set a limit on the number of states.

@wainwrightmark wainwrightmark changed the title added history implementation and example Feature: History/Undo/Redo May 20, 2023
@intendednull
Copy link
Owner

Looks great! Thank you for your contribution

@intendednull intendednull merged commit 51718ad into intendednull:master May 26, 2023
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.

2 participants