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

fix: add copy, rename, and delete as unsaved actions properly #1091

Merged
merged 3 commits into from
Dec 30, 2024

Conversation

jotalis
Copy link
Contributor

@jotalis jotalis commented Dec 29, 2024

Summary

  • Copying, renaming, and deleting schedules are now stored as unsaved actions and can be re-loaded if the user wants to
  • Fixes "an issue where duplicating a schedule that was not currently selected would incorrectly create a new schedule containing the events of the currently selected schedule, instead of duplicating the intended schedule," a bug also fixed in feature/drag-and-drop schedules #1089; will fix merge conflicts after either one gets merged into main
  • Slight code refactoring for consistency

Test Plan

  • Create several schedules with some events. Duplicate, rename, and delete some. Take note of the order and state of these schedules. Reload the page without saving these changes and load the unsaved changes. Ensure the loaded changes match the schedule states prior to reloading

Closes #1090

@jotalis jotalis requested a review from KevinWu098 December 30, 2024 06:54
Copy link
Member

@KevinWu098 KevinWu098 left a comment

Choose a reason for hiding this comment

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

LGTM, great feature improvement, however there's still a bit of bugginess as newly created schedules aren't logged in the action states, and thus actions upon newly created schedules then don't get applied

apps/antalmanac/src/components/dialogs/AddSchedule.tsx Outdated Show resolved Hide resolved
@KevinWu098 KevinWu098 self-requested a review December 30, 2024 09:47
Copy link
Member

@KevinWu098 KevinWu098 left a comment

Choose a reason for hiding this comment

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

LGTM, just the one comment above

Copy link
Member

@KevinWu098 KevinWu098 left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@jotalis jotalis merged commit 7560d46 into main Dec 30, 2024
4 checks passed
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.

fix: add delete and rename schedule as unsaved actions that can be restored
2 participants