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

[Feature] Color fields #329

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

[Feature] Color fields #329

wants to merge 4 commits into from

Conversation

usulpro
Copy link
Collaborator

@usulpro usulpro commented Feb 6, 2021

This PR adds a support for color fields

here is an example https://react-theming.github.io/storybook-addon

@mac-s-g
Copy link
Owner

mac-s-g commented Feb 8, 2021

i noticed an issue with collapsed strings:

image

color strings get collapsed but expanding them doesn't work.

you can see this in the first example on your branch if you npm run dev.

should we just ignore the collapsed string length for colors?

@mac-s-g
Copy link
Owner

mac-s-g commented Mar 8, 2021

I love this idea, thanks again for posting

i think there are a couple of things that should happen before this gets merged and published:

  1. add at least one test to make sure the color box is rendering (or not rendering) when appropriate
  2. i think there should be a prop that enables/disables this functionality. if the prop is disabled, we just treat colors as strings. I could see this feature being unexpected (possibly undesirable) for people using rjv to interact with their data. I'm definitely open to discussion on this.
  3. last of all, include a bump to the minor version in package.json.
  4. (optionally) update the demo src to include a color. this is a cool feature that would be fun to show off. then run npm run build:demo and add the updated demo build to the pr. I can merge into the gh-pages branch if you decide to do this.

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.

2 participants