🐛Fix: keep event draft during save process #181
Conversation
This reverts commit ec438d7.
There was a problem hiding this comment.
Hey @murilo9, I'm afraid the current state of this PR would do more harm than good due to the following issues I'm seeing
- When creating a new timed event, the title flashes indefinitely. This doesn't happen every time, but often enough.
new.flash.mov
- The form now stays open until the frontend receives a response from the backend. Previously, the form closed immediately. This delay causes a user to question whether they clicked the Save button.
- Clicking the empty space in the all-day row results in the event rectangle rendering, but not the form. Clicking the empty rectangle will then open the form. (This is occasional and may not be caused by this PR).
Please spend a little time considering if there's a way we can change our approach to fix these issues without having to do a big refactor. If not, I'd rather put this issue on hold until after cleaning up our state management than introduce more bugs.
| if (preservedDraft && !isDrafting && !isProcessing) { | ||
| return null; | ||
| } | ||
| if (!preservedDraft && !draft) return null; |
There was a problem hiding this comment.
This effect is probably what's getting us into trouble. But I'm wondering if draft is changing too frequently for this copied preservedDraft to stay in sync
|
Regarding no. 1 and 3, I'll give a look on them. For no. 2, may I add a loading state to the Save button (disable the button and change its text to "Saving...")? |
|
Adding a pending message ('Saving...') and disabling the button would mitigate confusion. But that's still a step backward compared to how it works now since we'd still be forcing the user to wait for the operation to finish. I'd rather focus on getting the event rectangle to always show without any strings attached. We could add some non-blocking pending state to show that the background operation is still processing as part of another enhancement. |
|
I thikn I don't really understand the logic of |
|
OK, appreciate the honesty. I'm going to close this so we can focus on other issues |
This fixes #145
What I've done:
Draft.tsx:useEffect's logic to sync preservedDraft with draft when isProcessing is false.How I've tested this fix: