-
Notifications
You must be signed in to change notification settings - Fork 2k
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 color picker display when updating an environment color #2711
Fix color picker display when updating an environment color #2711
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems alright but I'd like to know how it was working before the Electron update or if the logic here changed recently.
packages/insomnia-app/app/ui/components/modals/workspace-environments-edit-modal.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've noticed that for any reason calling
await this._load(this.state.workspace)
just after appending the input element to the body is fixing the display of the color picker.
I'm guessing that adding an await to something after appending the input element, is allowing some background process to complete.
I doesn't feel like forcibly updating the environment color each time we prompt for a new color is fixing the root cause.
I was able to get the same fix working by delaying the element click using setTimeout(() => el.click(), 50);
, so needs further investigation to determine and fix the root cause.
Needs further investigation as always updating the environment colour is fixing a symptom and not the root cause.
2f84384
to
a5aa16a
Compare
a5aa16a
to
10787e3
Compare
packages/insomnia-app/app/ui/components/modals/workspace-environments-edit-modal.js
Outdated
Show resolved
Hide resolved
84d441d
to
9af80e8
Compare
9af80e8
to
232bfde
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for updating the description and adding in a gif. Almost there!
packages/insomnia-app/app/ui/components/modals/workspace-environments-edit-modal.js
Outdated
Show resolved
Hide resolved
packages/insomnia-app/app/ui/components/modals/workspace-environments-edit-modal.js
Outdated
Show resolved
Hide resolved
232bfde
to
5d02d05
Compare
packages/insomnia-app/app/ui/components/modals/workspace-environments-edit-modal.js
Outdated
Show resolved
Hide resolved
5d02d05
to
d07264c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicely done 👏
We were removing the color picker to reset event handlers easily (fix #811).
It is possible to remove the listener quite easily by listening to the
change
event triggered when the color picker modal is closed.There was still an issue with the 1st click on
Change color
. We were adding theinput
dynamically on the 1st click and we were having some race condition between theclick
and theappendChild
🤔To fix that, I've added the
input
element just before theColor
dropdown. Besides, now we can open the color picker modal near the dropdown by making theinput
element invisible.Before
After
Closes #2647