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

Fixed restore_votes not update MyVotes #2991, #2986 #3868

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

mkbeefcake
Copy link
Contributor

@mkbeefcake mkbeefcake commented Nov 21, 2022

Added customEvent mechanism inside the pioneer and refresh the Election page after Restore Votes clicked.
Fixed #2991, #2986

@vercel
Copy link

vercel bot commented Nov 21, 2022

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

Name Status Preview Updated
dao ✅ Ready (Inspect) Visit Preview Dec 20, 2022 at 11:24AM (UTC)
pioneer-2 ✅ Ready (Inspect) Visit Preview Dec 20, 2022 at 11:24AM (UTC)
pioneer-2-storybook ✅ Ready (Inspect) Visit Preview Dec 20, 2022 at 11:24AM (UTC)

Copy link
Contributor

@WRadoslaw WRadoslaw left a comment

Choose a reason for hiding this comment

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

Hey! Nice hook logic!

Although I have a question. I don't see any other usage of this hook beside changes in local storage. Could we maybe instead of creating a new hook to dispatch new events, improve useLocalStorage logic to capture changes to itself from other places?

Here is a similar solution to yours, but only on local storage. Link

@mkbeefcake mkbeefcake changed the title Fixed restore_votes not update MyVotes #2991 Fixed restore_votes not update MyVotes #2991, #2986 Dec 5, 2022
@mkbeefcake
Copy link
Contributor Author

Hey! Nice hook logic!

Although I have a question. I don't see any other usage of this hook beside changes in local storage. Could we maybe instead of creating a new hook to dispatch new events, improve useLocalStorage logic to capture changes to itself from other places?

Here is a similar solution to yours, but only on local storage. Link

I tried to migrate customEvent inside useLocalStorage() hook. it applies the storage updated event to all states which uses same storage key. Please check and give me your feedback

Copy link
Member

@thesan thesan left a comment

Choose a reason for hiding this comment

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

Really nice solution 🎩

Just to 2 details:

return () => {
document.removeEventListener(`storage_event_${key}`, handleEventOnce)
}
}, [])
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if the key is ever changed somewhere in the code base, but to at least be consistent with other hooks dependencies called here:

Suggested change
}, [])
}, [key])

document.removeEventListener(`storage_event_${key}`, handleEventOnce)
}
}, [])

const dispatch = useCallback(
(setStateAction: T | ((prevState?: T) => T)) => {
const value = isFunction(setStateAction) ? setStateAction(getItem(key)) : setStateAction
setState(value)
Copy link
Member

Choose a reason for hiding this comment

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

Just to prevent redundant re-renders:

Suggested change
setState(value)

@dmtrjsg
Copy link
Contributor

dmtrjsg commented May 11, 2023

@chrlschwb why did we decide to close it? Looks like solution was nice and we were close to resolution.. if its due to no access to the branch @thesan could take this over at some point. Please let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-dev issue suitable for community-dev pipeline
Development

Successfully merging this pull request may close these issues.

[USERSNAP] Restore Votes does not upate My Votes
7 participants