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: Disable newly created event from being editing during optimistic render cycle #206

Conversation

that-one-arab
Copy link
Contributor

Description

While you already proposed a solution for this issue here, preventing the user from initially DnD or editing the event did not seem very UI/UX friendly to me.

I thought about and researched other solutions, the best solution to handle this issue is to implement Transactional State Management to handle this type of transient data, but this implementation itself is split into many strategies.

Each strategy has its own pros and cons, but they all share 1 same con and that is they are difficult to implement and adds lots of overhead.

The 2nd best approach was to simply display a loading indication when the event is initially being created, removing the optimistic approach entirely.

But this would:

  • Undo some of the work we did
  • Conflict with a comment you gave here

So the best thing we can do here is to implement your proposed solution which is disabling edits during its optimistic status.

I tried my best to make it UI/UX friendly by displaying a different cursor and a distinct background color to indicate we are doing some loading to the user.

I included all this extra context as logs for future reference and your own consideration.

Demo

Peek.2024-12-28.16-31.webm

… render cycle

Prevents the event from being modified when we initially insert the event as an optimistic event
@that-one-arab that-one-arab marked this pull request as ready for review December 28, 2024 13:42
the previous solution worked fine, but it came with the overhead of
another color map in theme.util. We're going to get
 away from these as we support more priorities,
 so this method lets us scale with more ease
@tyler-dane
Copy link
Contributor

Thank you for sharing your thought process, @that-one-arab. I like how your solution is simple yet also offers an intuitive UX. I especially like how the styles only change onHover, allowing users to not have to think about what's happening behind the scenes.

I followed up with one cleanup commit. The original implementation worked fine, but it came with the overhead of another color map in theme.util. We're going to get away from these as we support more priorities, so this method lets us scale with more ease

@tyler-dane tyler-dane linked an issue Dec 29, 2024 that may be closed by this pull request
@tyler-dane tyler-dane merged commit a6cb22e into SwitchbackTech:main Dec 29, 2024
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.

Event can still be changed during optimistic render
2 participants