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

[Autocomplete] Show an integration example #7477

Merged
merged 2 commits into from
Jul 20, 2017
Merged

[Autocomplete] Show an integration example #7477

merged 2 commits into from
Jul 20, 2017

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Jul 19, 2017

Closes #4783 and all the issues associated.

I think we would better off by not porting the component. Instead, documenting how to use external libraries like react-autosuggest or providing an adapter. That goes into the strategy of having the v1 less opinionated and closer to our core business. The 50+ opened issues shows that building such component is hard, let's increase the synergy but showing how to use styleless libraries to solve the it.

auocomplete

cc @callemall/material-ui

  • PR has tests / docs demo, and is linted.
  • Commit and PR titles begin with [ComponentName], and are in imperative form: "[Component] Fix leaky abstraction".
  • Description explains the issue / use-case resolved, and auto-closes the related issue(s) (http://tr.im/vFqem).

@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation v1 labels Jul 19, 2017
@oliviertassinari
Copy link
Member Author

@wieseljonas As been building his own integration at the same time:

That's confirming my thought that it's the best path to follow

@rosskevin
Copy link
Member

I agree, if there are any HOC's etc we could provide to aid in integration for any boilerplate code I would like to make those available.

If it is simply too specific/brittle etc I also understand, and we should not provide additional helpers due to the magnitude of issues that could arise.

Also, if someone wants to take a shot at maintaining this individual component separately, we would be happy to link to them etc.

renderInputComponent={renderInput}
suggestions={this.state.suggestions}
// suggestions={suggestions}
// alwaysRenderSuggestions
Copy link
Member Author

Choose a reason for hiding this comment

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

oops

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Jul 20, 2017

@rosskevin Yeah, I have been thinking about providing a MuiAutosuggest component, but having a look back at the demo, I don't see much opportunity to provide value with such abstraction. The Material-UI component part is straightforward (at the exception of the ref trick on the input).

@oliviertassinari oliviertassinari merged commit c2c4a72 into mui:v1-alpha Jul 20, 2017
@wieseljonas
Copy link
Contributor

@oliviertassinari that's great good find. I had used ListItem instead of MenuItem but I guess it's for the user to choose.

I'm haven't tested extensively with mobile but I'll get to it!

@oliviertassinari oliviertassinari deleted the autocomplete-demo branch July 20, 2017 10:54
@oliviertassinari
Copy link
Member Author

@wieseljonas I think that MenuItem is better suited for the use case, but users are definitely free to choose. I haven't noticed any issue yet on mobile, let us know.

@wieseljonas
Copy link
Contributor

@oliviertassinari seems to be working well. I've been using this with formik and relay for my forms it all work like a charm bye bye redux and redux-forms

@arracso
Copy link

arracso commented Jul 24, 2017

Hi, I remodeled the example provided using react-autocomplete so I can use it in different situation (even with async requests and allowing controlled/uncontrolled).
Here is my code, feel free to use it.
As you can see there is an implementation of SelectField, but it needs more work.

Ok now lets look at SuggestField. To use it, you only need to pass him getSuggestions and getSuggestionValue. If you want to use it as a controlled input, also pass it the value and the onChange.

An finally you can make async requests on the getSuggestions. Setting async to true.

If you want to style the textField just pass to SuggestField any prop that TextField can handle. (Except classes, use inputClasses instead)

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Jul 24, 2017

@arracso Thanks for sharing, we could use your SelectField implementation as a starting point for our own. At least, that's providing a better understanding of the landscape of the solutions available :).

@rosskevin rosskevin mentioned this pull request Jul 29, 2017
4 tasks
@rosskevin
Copy link
Member

Here is a gist of @oliviertassinari's integration sample genericized as much as possible. All the pluggable methods are defaulted but overridable. I'm not sure this is suitable to include here, but thought I would offer it to others. Resulting use is simply:

const suggestions = [
  { label: 'Afghanistan' },
  { label: 'Aland Islands' }
}

function getSuggestions (value) {
  return filterByLabel(value, suggestions)
}

const AutoCompleteBasic = () => (
  <AutoComplete
    getSuggestions={getSuggestions}
    placeholder='Search a country (start with a)'
  />
)

@ctavan
Copy link
Contributor

ctavan commented Jul 31, 2017

@arracso if you're interested in a SelectField component, please also have a look at #7458

Would be happy to incorporate your feedback in there.

@rosskevin rosskevin mentioned this pull request Jul 31, 2017
3 tasks
@rosskevin
Copy link
Member

Just for the record, I've been integrating our own take on AutoComplete into our app code. I'm in full agreement with @oliviertassinari at this point. There are too many decisions to be made to have a generic AutoComplete in material-ui. I think any attempt at including one would be inadequate for many and too complicated for most, so it is best to leave as an integration sample.

@ctavan
Copy link
Contributor

ctavan commented Aug 1, 2017

@rosskevin I totally agree.

Have you ever considered publishing the contents of your gist as a module on npm? While I agree that use cases vary largely when it comes to AutoComplete, I still believe that there are probably many people for whom your solution would be just good enough and who would appreciate if it could easily be added as an npm dependency.

(Side note: I once published the contents of a gist as an npm package, which then evolved for more than 6 years and has become quite popular… So even though I myself haven't used it in the last 5 years there were apparently plenty of people who appreciated it and other people who were willing to continue maintaining it.)

@rosskevin
Copy link
Member

I published above, and might publish again because it is getting more complete...but seems like too many decisions to be a general use package.

@arracso
Copy link

arracso commented Aug 1, 2017

@ctavan I made a SuggestField usign react-autocomplete. It allows async and sync sugestions. Here you have the code. I don't know how to do npm packages. Feel free to do watherver you want with it.

@rosskevin
Copy link
Member

Here is my complete work on my AutoSuggest based on @oliviertassinari's integration example.

This one is enhanced by:

  1. Using react-popper and react-travel to portal the suggestions container - preventing any ancestor overflow: hidden problems
  2. Using the Grow transition extracted from Menu/Popover - depends on PR [Popover] Refactor popover transition - separation of concerns #7720

react-popper/Popper.js is awesome because as you can see, it will fallback to different positions if the preferred position bottom-start doesn`t have room:

profile_-_dummy_com

Here's my gist:
https://gist.github.com/rosskevin/1bd998940f8e9decb85aa624e45ed2d9

If someone is feeling helpful, it would be fantastic if my work on the react-popper was integrated back into our project demo. I think it is a must-have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants