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] Provide an all in solution #13863

Closed
2 tasks done
shadabk96 opened this issue Dec 9, 2018 · 23 comments · Fixed by #17037
Closed
2 tasks done

[Autocomplete] Provide an all in solution #13863

shadabk96 opened this issue Dec 9, 2018 · 23 comments · Fixed by #17037
Assignees
Labels
component: autocomplete This is the name of the generic UI component, not the React module! new feature New feature or request priority: important This change can make a difference
Milestone

Comments

@shadabk96
Copy link

A lot of additional work needs to be done, in order to provide material-ui styling to default react-select components. Can we have a wrapper select component with m-ui look and feel which implements react-select underneath.

  • This is not a v0.x issue.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior 🤔

  • Should be able to use options to populate options, similar to react-select.
  • Should have m-ui look and feel. Check autocomplete example

Current Behavior 😯

N/A

Examples 🌈

  • Check autocomplete demo

Context 🔦

  • Users would be able to leverage react-select features without the need to explicitly provide m-ui styles.
@oliviertassinari oliviertassinari added waiting for 👍 Waiting for upvotes new feature New feature or request labels Dec 10, 2018
@oliviertassinari
Copy link
Member

I have mixed feeling about it. I'm adding the waiting for users upvotes flag.

@amesas
Copy link

amesas commented Dec 11, 2018

+1 for it.

Material-UI is great providing building blocks, but we would like that, once all basics components are "done", start with more complete components.

We switch daily from Material-UI to Vuetify, for different projects, and we feel more productive with Vuetify: autocomplete is on of those cases. It's already built in. But we could give more: rating, timeline, treeview, combobox, sparkline calendar, etc..

Not only new components, but the existing ones have more features built-in, giving a great DX.

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 11, 2018

@amesas This is a great feedback. If you can share specific points where you feel more productive, it would help a lot prioritizing our effort.

@SladeRun14
Copy link

We'd like to suggest that offering a Select component without auto-completion/filtering functionality doesn't make much sense today as any modern user expects that functionality. We also find that implementing this functionality with Material-UI is more work than it should be, especially when there are so many other frameworks that offer it out of the box (Ant Design, Vueify, among others).

Add to this (while I'm at it :)) the need to occasionally wrap a select component in a redux-form field and the work really multiplies when using Material-UI.

For what it's worth, we like react-select's approach, and think the community would gladly accept an opinionated Material UI SelectField based on it.

In a nutshell, it would be nice if we could create a basic (redux-form-compatible) Material-UI SelectField as easily we do with native react-select component:

Class ReactSelectReduxForm extends React.PureComponent {

  render() {
    const { input, label, placeholder, items, meta, className, ...StyledProps } = this.props
    const { onChange, onBlur, onFocus } = this.props.input

    return (
      <div >
        <Select
          name={input.name}
          value={input.value}
          onChange={input.onChange}
          onBlurResetsInput={false}
          onCloseResetsInput={false}
          onFocus={onFocus}
          options={items}
          {...this.props}
        />
      </div>
    );
  }
}

All that said, Material UI is still our preferred framework and will be for the foreseeable future. Thank you for your valuable contribution to this space...

A rather long-winded +1

@amesas
Copy link

amesas commented Dec 15, 2018

@oliviertassinari , I will add more detailed info, but it's on @SladeRun14's boat.
It feels like always adding the same "basic features" to M-UI components.

M-UI is getting enough mature to start adding/improving some components, that i think can be considered basic nowadays.

Ratings, Tree views, Timelines, Fileupload, feature discovery, sparklines, pickers, combined pickers, calendar...

Material Design is not only "how to paint a component", but it defines UX, and those more complex components could help devs get this Material Design UX.

Right now when you start using M-UI and go beyond some basic forms, you start looking for tons of external packages, and trying to guess how to use them with M-UI.

I'm not talking about having a DevExtreme React Grid into M-UI, but some components feel pretty basic like in this case: select/autocomplete (Why we need an external component?). M-UI table should be able to filter/search without adding an external package, etc.

@oliviertassinari oliviertassinari changed the title M-UI wrapper for react-select [Autocomplete] Wrapper for react-select Mar 13, 2019
@TrejGun
Copy link
Contributor

TrejGun commented Apr 8, 2019

I want to add my 2 cents as developer

we are using material-ui with react-select for several different cases and i want to point to edge cases which are important for us

  1. integration with forms helpers. it was mentioned that people are using it with redux-form and we are using it with final-form. feels really good.

  2. in your current exaple styles are not matching material ui styles. by this i mean little down arrow is different from simple Select

  3. there is ambiguousness in how to pass props to input using your example

     <Select
        autoComplete="off"
        textFieldProps={{
          autoComplete: "off",
          inputProps: {
            autoComplete: "off",
          },
        }}
      />

this should be documented beeter especially looking at next case

  1. awesome browser chrome does not give a shit about autoComplete property on input field, you can google about details, but long story short, now correct way to turn off autocomplete is to set in to some inproper string like "disabled" so passing down this property is something important for users whos implementing field with country/city selection

  2. you cant pass autoComplete property in any of the ways mentioned above because real input field is not created by material ui, the only way to pass this property is to implement Input component yourself

import React from "react";

import {components} from "react-select";

function Input(props) {
  return (
    <components.Input {...props} autoComplete="disabled" />
  );
}

export default Input;

you probably can add this to docs as gotchas

  1. react-select is not in a phase of the active development and has 1000 open bugs. It is not critical but still

looking at all this you probably can reimplement all components with material components. this also will remove all unused dependencies from bundle of end user. we absolutely dont need emotion and react-input-autosize

@oliviertassinari oliviertassinari added the priority: important This change can make a difference label Apr 30, 2019
@oliviertassinari oliviertassinari changed the title [Autocomplete] Wrapper for react-select [Autocomplete] Provide an all in solution Apr 30, 2019
@oliviertassinari oliviertassinari added this to the v5 milestone Apr 30, 2019
@brianle1301
Copy link
Contributor

brianle1301 commented Jun 26, 2019

@oliviertassinari I want to add something here. It's not just for convenience' sake, but also for the stability and ability to move fast of the framework/library (whatever it's called). Take this issue as an example #16364. Material-UI's <Select /> cannot be used with react-virtualized (<List />), limiting the versatility of the component but also generate an unnecessary issue for the team, making us come back to address this in the future. Sometimes, this incompatibility might be a pain for both users and maintainers, having us looking back at not only our source but also others. By rewriting some of the components (hence providing new APIs) and adding some of the mentioned combo-boxes, we have full control over the state of the library, which in my opinion provides better bug fixes and updates in the future as we no longer need to care about other dependencies' API.

If this becomes a possibility, count me in. I want to help with this, also I might need some guidance. And correct me if I am wrong. Cheers! 👍

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 26, 2019

@dreamsinspace This issue is one of the most important one open right now on the project. I used to believe that providing different customization options to users would be the best approach: https://material-ui.com/components/autocomplete/. However, after reviewing the user survey answer we have run at the beginning of the year. I'm no longer sure about that. People seem to really value the one-stop component store proposition of Material-UI.

I'm also amazed by how much react-select grows while being hard to customize (I have tried in our demos), having a none negligible bundle size cost, over 1k opened issues, numerous accessibility issues and not feature very rich.

So going forward, I think that the best option is to ship our own components with minimal dependencies. We could provide one/multiple components that support different use cases:

I haven't looked that the dependencies of the benchmarked solutions, but I would guess most do it from scratch (to be verified).

Any help would be highly appreciated! (We could even consider paying $ to fund such effort.)

@mbrookes
Copy link
Member

mbrookes commented Jun 26, 2019

I happened to come across this blog post today (while searching for something completely unrelated to this issue).

The component in question might prove a useful reference for accessibility (and code?). It is 10.8KB with only one runtime I dependency - preact.

@brianle1301
Copy link
Contributor

brianle1301 commented Jun 27, 2019

@mbrookes Never knew there's a blog that actually mentions that kind of information on gov.uk! 🤣
@oliviertassinari I think starting from scratch would be a great idea. You could maybe open some sort of issues to begin with, and list out all the use cases and scenarios. I'm quite new to contributing to this repo, but I think given the above information I can do pretty well

@eps1lon
Copy link
Member

eps1lon commented Jun 27, 2019

The component in question might prove a useful reference for accessibilit

Or we can cut the middleman and start with WAI-ARIA which they did as well:

Accessibility: Following WAI-ARIA best practices and testing with assistive technologies.

If we look at their implementation first we might as well just use their component and apply our styling. No need to waste time re-implementing it and calling it our own. See also internal doc about a11y implementation in material-ui.

@mbrookes
Copy link
Member

Or we can cut the middleman and start with WAI-ARIA which they did as well

ARIA isn't the be-all and end-all of accessibility. They didn't just just consult the specs, they consulted accessibility experts.

@eps1lon
Copy link
Member

eps1lon commented Jun 27, 2019

ARIA isn't the be-all and end-all of accessibility.

No need to argue against claims nobody made.

They didn't just just consult the specs, they consulted accessibility experts.

No but they put it first and those experts probably contributed to the WAI-ARIA docs.

@brianle1301
Copy link
Contributor

brianle1301 commented Jun 30, 2019

@oliviertassinari I made a start on <AutoSuggest /> component using a combination of <Popper />, <TextField />, <Paper />, <ClickAwayListener />, <Fade /> and <MenuItem />. A lot more needs to be done in order to provide more customisation for users and improve performance, but this is what I came up with: https://codesandbox.io/s/loving-gould-2dz5d.
Edit: Also, I'm not the best with accessibility, but I did some research and tried adding some aria- attributes.
Further edit: Since popperKey and suggestions state change quite frequently, we can introduce a useDebounce hook like so:

import { useState, useEffect } from 'react';

export default (value, delay) => {
  const [debouncedValue, setDebouncedValue] = useState(value);

  useEffect(() => {
    const handler = setTimeout(() => {
      setDebouncedValue(value);
    }, delay);

    return () => {
      clearTimeout(handler);
    };
  }, [value, delay]);

  return debouncedValue;
};

And apply it

// The durations need to be kept the same, otherwise <Popper />'s re-calculation won't sync with the suggestions
const debouncedSuggestions = useDeboune(suggestions, 500);
const debouncedPopperKey = useDebounce(popperKey, 500);

@oliviertassinari oliviertassinari self-assigned this Jul 13, 2019
@brianle1301
Copy link
Contributor

@oliviertassinari Can u re-assign this to me? I'm pretty much done with prototyping and will open a PR in about 2 days.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 16, 2019

@dreamsinspace Sure :), can we have a brainstorming session at some point, maybe on Gitter? I think that the sooner the better. I think that it's an important and great opportunity to help our users.

@Toub
Copy link

Toub commented Aug 14, 2019

@dreamsinspace did you open a PR? If yes, could you post the link here? Thanks!

@brianle1301
Copy link
Contributor

@Toub @oliviertassinari Hey folks. Sorry for the delay. I have the documentation and the component ready, just really trying to figure out how to do tests and type definitions. I'm kind of lost since MUI uses a lot of testing utilities and complex types under the hood. If possible, @oliviertassinari it would be nice if you could guide me through this. I tried DMing you on Spectrum, but didn't get a reply :p

@joshwooding
Copy link
Member

@dreamsinspace It might be better to create a Draft PR with the work you have already done, that way people can see it and it will make it a lot easier for everyone to help.

@oliviertassinari
Copy link
Member

Oops, sorry, I had disabled the notifications on Spectrum side. (I wasn't very happy with the way it was turning out: a split of the community. I think that we should consolidate it on StackOverflow, at least for generic question/answer threads.)

@Toub
Copy link

Toub commented Aug 15, 2019

@dreamsinspace, I agree with @joshwooding, a draft PR is the best place to discuss and contribute to your work in progress!

@brianle1301
Copy link
Contributor

@Toub @oliviertassinari @joshwooding Alright folks. Will make a draft PR if I get some free time tomorrow. Cheers :)

@brianle1301
Copy link
Contributor

@Toub @oliviertassinari @joshwooding PR created. Please give it a look 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: autocomplete This is the name of the generic UI component, not the React module! new feature New feature or request priority: important This change can make a difference
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants