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: use Cascader component that will be released in v7.5.0 #33

Merged
merged 5 commits into from
Mar 15, 2021

Conversation

vickyyyyyyy
Copy link
Member

@vickyyyyyyy vickyyyyyyy commented Mar 12, 2021

We need the latest updates to the Cascader component from these PRs:

but we don't want to have to bump the minimum required Grafana version to 7.5.0, where it will be released.

More info in README

@srclosson
Copy link
Member

Not sue what feedback you are looking for. I think it looks great. Lots of tests, storybook, documentation... Maybe a a few commented lines that could be removed? But it looks excellent!

@vickyyyyyyy
Copy link
Member Author

Not sue what feedback you are looking for. I think it looks great. Lots of tests, storybook, documentation... Maybe a a few commented lines that could be removed? But it looks excellent!

Whether this approach of us adding the components into this project seems good over the canary release solution (and any other possible solutions/workarounds we could use)? And if the added details in the README make sense or if I've missed anything out? 😄

@vickyyyyyyy vickyyyyyyy marked this pull request as ready for review March 12, 2021 14:26
@srclosson
Copy link
Member

Yup, read through the readme, and I think it's great. You understand the problem well, and make it clear when to use ui-enterprise. I like that you set expectations around the lifecycle of a component, explaining that it's a requirement to eventually have it merged into grafana-ui, and when it is, what the contributor should do for the code. 👍

Copy link
Member

@srclosson srclosson left a comment

Choose a reason for hiding this comment

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

LGTM 🔥

@vickyyyyyyy vickyyyyyyy merged commit 874e587 into master Mar 15, 2021
@vickyyyyyyy vickyyyyyyy deleted the cascader-7.5.0 branch March 15, 2021 10:59
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.

None yet

2 participants