Refactor useBackupReminder hook #528
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
danditomaso
force-pushed
the
add-dismiss-to-key-reminder
branch
from
fb67e90 to
22dbfbc
Compare
danditomaso
changed the title
|
|
I believe it is, the only people who access both local dev and live are developers. This is really a DX improvement without any downside for users. |
| > | ||
| Remind me in {reminderInDays} days | ||
| </Button> | ||
| <div className="flex flex-col gap-2"> |
There was a problem hiding this comment.
❔ This could/should be a form for better semantics/accessibility
There was a problem hiding this comment.
Yeah, makes sense
There was a problem hiding this comment.
Might be an interesting "future" thing to see if this could be converted to the new(ish) popover stuff
| suppressReminder(reminderInDays); | ||
| }} | ||
| > | ||
| Remind me in {reminderInDays} days |
There was a problem hiding this comment.
maybe
| Remind me in {reminderInDays} days | |
| Remind me in {reminderInDays} day{reminderInDays > 1 ? 's' : ''} |
There was a problem hiding this comment.
Fixed, I added this into the refactor I did based on this feedback. Thanks
| ); | ||
| setReminderState({ | ||
| suppressed: true, | ||
| lastShown: new Date().toISOString(), |
There was a problem hiding this comment.
did you mean to do this?
| lastShown: new Date().toISOString(), | |
| lastShown: new Date().toISOString(), | |
| expires: expiryDate.toISOString() |
There was a problem hiding this comment.
Thanks for catching this, I'm not sure why I thought I needed this extra logic, I did a small refactor using expires instead of supressed and lastShown props.
danditomaso
force-pushed
the
add-dismiss-to-key-reminder
branch
from
6d00b60 to
a7a448c
Compare
Description
This PR refactors the useBackupReminder hook to use localStorage for persistence instead of relying on cookies (js-cookie). This change improves the reliability of storing reminder state, and will allow a single dismiss action to be read across domains (local dev vs live)
Using cookies limited persistence to the specific domain and environment where the reminder was dismissed. By moving to localStorage, the dismissed state can now persist across local development and live environments (when the same storage is accessible), improving the developer experience and preventing repeated backup reminders between local development and live.
Changes Made
Displays the backup reminder toast based on the last shown date and suppression state.
Allows users to dismiss the reminder forever (10 years), take action, dismiss for one week.
Testing Done
Tests have been added for useToast and useLocalStorage hooks, including the Toast component
Screenshots (if applicable)
(the size of the dialog has remained the same fyi)
before:
after:
Checklist
Additional Notes