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 a Async Select that fetches options from given endpoint #1909

Merged
merged 2 commits into from
Jan 11, 2017

Conversation

vera-liu
Copy link
Contributor

@vera-liu vera-liu commented Jan 5, 2017

Done:

  • added a AsyncSelect component that fetches options from data endpoint, structure is similar to TableLoader
  • use AsyncSelect for users and databases (query search page and sql editor left)
  • replace DatabaseSelect with AsyncSelect
  • added specs

UI stays the same as in old Select:
screen shot 2017-01-05 at 2 50 55 pm
screen shot 2017-01-05 at 2 51 06 pm

needs-review @ascott @mistercrunch @bkyryliuk

wrapper.find('[name="select-user"]')
.simulate('change', { value: 1 });
expect(wrapper.state().userId).to.equal(1);
expect(wrapper.find(Select)).to.have.length(3);
Copy link

Choose a reason for hiding this comment

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

should this assert 3 or 4 selects? it was 4 before this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

after we switched the user select to AsyncSelect it became 3

Copy link

Choose a reason for hiding this comment

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

ok cool. we should update the it statement on L21 to read should have three selects in that case.

render() {
return (
<div>
<Select
Copy link

Choose a reason for hiding this comment

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

what do we show when the select is loading data?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

isLoading is true when loading the data, Select will render a spinner

@ascott
Copy link

ascott commented Jan 11, 2017

LGTM after updating the it statement in the spec.


const defaultProps = {
placeholder: 'Select ...',
value: null,
Copy link
Member

Choose a reason for hiding this comment

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

I think null defaults are implicit.

@mistercrunch
Copy link
Member

Neat! This reusable component is super useful to have!

@vera-liu vera-liu merged commit f0917c6 into apache:master Jan 11, 2017
SalehHindi pushed a commit to SalehHindi/superset that referenced this pull request Jun 9, 2017
)

* Add a Async Select that fetches options from given endpoint

* update it statement
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.15.2 labels Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.15.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants