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

Changes to Saturation/Hue.vue to isolate X/Y Events and Fix #113 #147

Merged
merged 2 commits into from
Aug 12, 2018
Merged

Changes to Saturation/Hue.vue to isolate X/Y Events and Fix #113 #147

merged 2 commits into from
Aug 12, 2018

Conversation

paulm17
Copy link

@paulm17 paulm17 commented Jun 28, 2018

This PR is to fix the issues in #113.

What I have done for both Saturation and Hue X/Y elements is to make them separate from the global colors variable and to use a local data variable.

With this change, the issue of the Hue slider shunting to the left when the pointer goes to the bottom AND the effect that I have seen. Is when the pointer goes near to the bottom, it sometimes itself shunts to the left or right.

Now this is not the case.

Finally, I've only had a limited amount of time to work on this. I'm sure the code could be refactored better. Maybe for 3.0?

Also please test and ensure this does not degrade the color data in anyway!

@linx4200 linx4200 changed the base branch from master to feat/2.5.0 August 12, 2018 08:44
@linx4200
Copy link
Collaborator

linx4200 commented Aug 12, 2018

I need to merge this into an origin branch, cause somehow I can't pull the branch locally.

@linx4200 linx4200 changed the base branch from feat/2.5.0 to fix/#113 August 12, 2018 08:46
@linx4200 linx4200 merged commit 7c63f03 into xiaokaike:fix/#113 Aug 12, 2018
@paulm17
Copy link
Author

paulm17 commented Aug 12, 2018

I completely forgot about this.

There is actually a regression that I introduced. The saturation does not change when the preselect colors are changed.

I also didn't fully resolve the issue back when the PR was submitted.

That said. Recently I solved the issues.

The unfortunately reality, is that I've had to fork the code significantly. If you want, I can put up the codebase somewhere for you to look at and then maybe you can figure it out and merge in the changes?

Let me know!

@linx4200
Copy link
Collaborator

linx4200 commented Aug 12, 2018

There is a problem in this PR.
The pointers in the Saturation Panels can't be synced across components, because the position of the pointer is decided in handleChange method.
Need to be decided by the value passed into the Saturation Panel.

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.

None yet

2 participants