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

feat(color-picker): Configurable color string format (HEX,RGB,HSL,HSV) #149

Merged
merged 12 commits into from
Aug 7, 2020

Conversation

motabass
Copy link
Contributor

@motabass motabass commented Aug 3, 2020

Implementing #140

Format is only for Output of the color-picker and internal string representations.

As Inputs (selectedColor, addColor, USED_COLORS) ALL formats supported by tinycolor are supported (https://github.com/thebespokepixel/es-tinycolor/blob/master/readme.md). They are then normalized to the selected COLOR_STRING_FORMAT internaly.

@motabass
Copy link
Contributor Author

motabass commented Aug 3, 2020

@tiaguinho BTW: what do you think about updating the dependencies?

@tiaguinho
Copy link
Owner

@motabass I think it's a good idea.
Can you open another PR these updates?

@tiaguinho tiaguinho self-requested a review August 5, 2020 18:40
@tiaguinho tiaguinho self-assigned this Aug 5, 2020
@tiaguinho tiaguinho added this to the v7.2.0 milestone Aug 5, 2020
@tiaguinho tiaguinho linked an issue Aug 5, 2020 that may be closed by this pull request
@motabass
Copy link
Contributor Author

motabass commented Aug 6, 2020

Sure.

@tiaguinho
Copy link
Owner

@motabass can you do a rebase with the master branch?

@motabass
Copy link
Contributor Author

motabass commented Aug 6, 2020

done

@tiaguinho
Copy link
Owner

tiaguinho commented Aug 7, 2020

@motabass there is a little bug.
Go to the first color-picker and choose a color.
Open the second color-picker. The icon of the selected color will be placed on the selected color from the first color-picker.
If you select a color in the second color-picker, the icon on the first will point to that color, but the selected color will not change.

@motabass
Copy link
Contributor Author

motabass commented Aug 7, 2020

I took a look at it.

I think that happens because of the service holding the _selectedColor. All collection components get this value OnInit and when updated.
I think the selectedColor should only exist on a "per color-picker" level.
@tiaguinho Do you know why it is implemented like that? Edit: I just found out myself...its for the custom color-collection added by users.

The solution should be to pass the selectedColor as an @input to color-picker-collection.component.ts

@motabass
Copy link
Contributor Author

motabass commented Aug 7, 2020

The Problem you described should now be fixed. I moved the selectedColor stuff into another service which is provided by the Component. So every Color-Picker has its own Service only for that

Thanks for the review!

@tiaguinho
Copy link
Owner

Thanks for fixing the bug.
Can you take a look and fix the tests?

@motabass
Copy link
Contributor Author

motabass commented Aug 7, 2020

done

@motabass
Copy link
Contributor Author

motabass commented Aug 7, 2020

I also finally added some event.preventDefault() to help with a strange behaviour i encountered sometimes when dragging the color selectors where the cursor changed to a blocked one and selection was prevented.

...and i changed the version shown in demo to 7.2.0
...and prevented some layout redraws in the browser by using transform instead of top and left positions
...and did some refactorings

@tiaguinho tiaguinho merged commit 6575cf0 into tiaguinho:master Aug 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

request(feature:color-picker) Support different color formats than HEX
2 participants