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

Reduce impact of ApiColorPicker emit bug #4328

Merged

Conversation

manuelmeister
Copy link
Member

Use default autoSaveDelay to fix #4212

@manuelmeister manuelmeister added Bug deploy! Creates a feature branch deployment for this PR labels Dec 26, 2023
Copy link

github-actions bot commented Dec 26, 2023

Feature branch deployment currently inactive.

If the PR is still open, you can add the deploy! label to this PR to trigger a feature branch deployment.

Copy link
Member

@usu usu left a comment

Choose a reason for hiding this comment

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

This doesn't fix the bug - it must makes it a bit harder to get the timing right in order to reproduce 😄

Recording.2023-12-26.191728.mp4

To me this smells like a wrong handling of dirty data.

  • outside value should not override local data if local data is dirty
  • outside value should not trigger an API call

@manuelmeister
Copy link
Member Author

@usu I agree, sadly the colorpicker always emits when the input value doesn't match the exact VColorPicker color object (which is never). This would need a wrapper component to encapsulate the colorpicker.

@manuelmeister
Copy link
Member Author

@usu I couldn't easily fix the picker problem. I did manage to prevent an api request if the value changes from the outside. I think we could fix the dirty problem in a separate PR.

@manuelmeister
Copy link
Member Author

@usu I think my PR already provides a real improvement. Could you have a look again?

@manuelmeister manuelmeister mentioned this pull request Jan 5, 2024
@manuelmeister manuelmeister changed the title Fix ApiColorPicker racing bug Reduce impact of ApiColorPicker emit bug Jan 6, 2024
Copy link
Contributor

@BacLuc BacLuc left a comment

Choose a reason for hiding this comment

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

The experience is better than before.
For most users it will be enough

@manuelmeister manuelmeister added this pull request to the merge queue Jan 8, 2024
@usu
Copy link
Member

usu commented Jan 8, 2024

@usu I couldn't easily fix the picker problem. I did manage to prevent an api request if the value changes from the outside. I think we could fix the dirty problem in a separate PR.

Opened a new issue for this at #4404

Merged via the queue into ecamp:devel with commit d86b4cd Jan 8, 2024
32 checks passed
@manuelmeister manuelmeister deleted the bugfix/#4212--api-colorpicker branch January 8, 2024 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug deploy! Creates a feature branch deployment for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ApiColorPicker does not work in autosave mode
4 participants