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

Add custom formatting for Dynamic Component #519

Merged
merged 3 commits into from
May 31, 2019

Conversation

codemayank
Copy link

  • Please check if the PR fulfills these requirements
  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    Feature Update.

  • What is the current behavior? (You can also link to an open issue here)
    Addresses Custom formatting for Dynamic #490

  • What is the new behavior (if this is a feature change)?
    Dynamic Component will accept a prop map that will map a give value to the value prop of the component. e.g.

[Dynamic value:Var1 min:50 max:100 step:10 map:`['Red', 'Red', 'Blue', 'Green']`/]
[Dynamic value:Var2 min:0 max:4 step:1 map:`(Var2 % 2 === 0) ? 'even' : 'odd' ` /]

This will be the output of above
custom_formatting

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
    No.

  • Other information:

@codemayank codemayank requested a review from mathisonian May 30, 2019 16:24
Copy link
Member

@mathisonian mathisonian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @codemayank! I've left a couple comments that I think will help make the API match the use case that @suhr and @rowanc1 were describing in #490

packages/idyll-components/src/dynamic.js Outdated Show resolved Hide resolved
packages/idyll-components/src/dynamic.js Show resolved Hide resolved
baryon2 added 2 commits May 31, 2019 07:39
Dynamic Component: handle negative step and 0 for arrays
-format prop accepts custom format function
@codemayank codemayank force-pushed the dynamic-custom-formatting branch from a1c9e1c to 9b27240 Compare May 31, 2019 02:15
@mathisonian
Copy link
Member

After thinking about it a bit more, passing in a reactive expression rather than a function seems more idyllic.

I pushed an update to make this work, and made a short test post implementing some of @rowanc1's examples. See here:

https://idyll.pub/post/dynamic-test-3f0ebd38f8d30970b1da2f89/

Since its not a function anymore and there isn't an easy way to differentiate it from our existing format option, I've changed it back to accept a second prop, display. See the example above to see this in action, I think this seems like a pretty nice API.

@mathisonian mathisonian merged commit 1b4c230 into master May 31, 2019
@mathisonian mathisonian deleted the dynamic-custom-formatting branch May 31, 2019 21:10
@rowanc1
Copy link

rowanc1 commented Jun 3, 2019

Thanks for implementing this @mathisonian and @codemayank!

I started a similar project before I discovered Idyll, I wrote up a bit of a review of the differences in my thinking/features so far:

curvenote/article#8

I would be very curious on your thoughts on this. Really awesome work on this project/language.

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.

4 participants