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

Formalize useColorPicker as a component export #2895

Closed
thompsongl opened this issue Feb 21, 2020 · 9 comments
Closed

Formalize useColorPicker as a component export #2895

thompsongl opened this issue Feb 21, 2020 · 9 comments
Assignees

Comments

@thompsongl
Copy link
Contributor

Looks like useColorPicker is probably a service we'll want to document and export if it's this helpful in a consuming application?

Originally posted by @cchaos in #2850


This idea extends beyond this case to other components with slightly-non-standard form element input-output expectations.

@Dilshaad21
Copy link

Dilshaad21 commented Feb 21, 2020

@thompsongl I want to work on this. Is it related to the src/docs part

@chandlerprall
Copy link
Contributor

chandlerprall commented Feb 22, 2020

@Dilshaad21 This change is stand-alone and wouldn't interact with other changes or parts of EUI; specifically it's moving useColorPicker (and useColorStop) from src-docs/src/views/color_picker/utils.js and into a new service file in src/services Feel free to take and work on it!

I'd recommend renaming this helper to something like useColorPickerState, as we've also used hooks to return actual Components themselves and can see the naming convention overlapping. Thoughts, @thompsongl ?

@Dilshaad21
Copy link

Ok @chandlerprall, going for it

@thompsongl
Copy link
Contributor Author

useColorPickerState is a good name. I haven't fully thought through all of the impacts this could have, but a relocation and rename seem like a good start.

@mridulgogia
Copy link
Contributor

@Dilshaad21 Are you working on this?

@mridulgogia
Copy link
Contributor

@thompsongl Can I work on this issue?

@thompsongl
Copy link
Contributor Author

I've made some modifications to the function in #2850. We should wait until that PR merges before moving forward again with work on this issue

@chandlerprall
Copy link
Contributor

#2850 has been merged, this is ready to be worked on @mridulgogia

@thompsongl
Copy link
Contributor Author

Done in #3067

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

No branches or pull requests

4 participants