search

evidence-dev / evidence

Business intelligence as code: build fast, interactive data visualizations in pure SQL and markdown
https://evidence.dev
MIT License
3.44k stars 167 forks source link

Input components #1668

Closed mcrascal closed 2 months ago

mcrascal commented 2 months ago

Breaks input components out of 1569

changeset-bot[bot] commented 2 months ago

🦋 Changeset detected

Latest commit: a51e5a33ae0e5e7d316bf4288018d6bb6088af54

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages | Name | Type | | ----------------------------- | ----- | | @evidence-dev/core-components | Patch | | @evidence-dev/evidence | Patch | | @evidence-dev/components | Patch | | evidence-test-environment | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

vercel[bot] commented 2 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 6, 2024 1:50am
netlify[bot] commented 2 months ago

Deploy Preview for evidence-development-workspace ready!

Name Link
Latest commit a51e5a33ae0e5e7d316bf4288018d6bb6088af54
Latest deploy log https://app.netlify.com/sites/evidence-development-workspace/deploys/65e7cbe790d9360008fcdd7f
Deploy Preview https://deploy-preview-1668--evidence-development-workspace.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

csjh commented 2 months ago

I'm just noticing/remembering I never concluded the dropdown backwards compatibility fix

I'm leaning on the side of an includeLabels prop since it seems cleaner for everyone involved

archiewood commented 2 months ago

how would that work?

<Dropdown includeLabels=true/> allows

${inputs.my_input.value} and ${inputs.my_input.label}?

but <Dropdown/>

only allows

${inputs.my_input}

csjh commented 2 months ago

yup

archiewood commented 2 months ago

Sounds like a good solution to me.

What do you think about warning a breaking change in the future?

${inputs.my_input} will be deprecated in later versions of evidence.

Use ${inputs.my_input.value} in combination with includeLabels=true

csjh commented 2 months ago

Yeah makes sense to me

mcrascal commented 2 months ago

@archiewood I'm inclined to just make the breaking change and announce it in the changelog

archiewood commented 2 months ago

That's fine. We should definitely add a major bump in that case