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 disableAdornment, disableLabel to getInputProps #21

Merged
merged 1 commit into from
Mar 6, 2018
Merged

add disableAdornment, disableLabel to getInputProps #21

merged 1 commit into from
Mar 6, 2018

Conversation

caub
Copy link
Contributor

@caub caub commented Mar 5, 2018

No description provided.

@techniq
Copy link
Owner

techniq commented Mar 5, 2018

@caub Instead of disableLabel how about we use the existance of a passed label to include or not...

{label && <InputLabel {...labelProps}>{label}</InputLabel>}

For disabledAdornment this should also be able to be accomplished with

<StarWarsSelect getInputProps={() => ({ endAdornment: null })} />

I'd rather leave go that route unless I'm missing something. After the change to label, this example should accomplish what you are looking for I believe.

@caub
Copy link
Contributor Author

caub commented Mar 5, 2018

Good points, thanks, done ^

@techniq techniq merged commit 1015cae into techniq:master Mar 6, 2018
@techniq
Copy link
Owner

techniq commented Mar 6, 2018

I just published this as 1.0.0-9 and can be installed using yarn add mui-downshift@next.

Once we have a good example story for multi-select (#5, including chips and "# selected") and option groups (#20) I plan to release 1.0.0. I've been holding off incase there were any breaking changes required to support those use cases.

Thanks for your contributions to the project 😄

@caub
Copy link
Contributor Author

caub commented Mar 6, 2018

@techniq you're welcome

for the multi-select, I'd avoid to toggle an item when selecting it again (rather filtering it from filteredItems, or removing and adding it in last position)

Also, it's not easy, but the dropdown should always be opened as long as the input has focus. Sometimes I have to blur and refocus it (or I could type in too) to reopen it after a selecting an item)

I'm working on the multi-group-select, but without downshift for now, in pure mui with react-popper, I'll post a codesandbox, and hopefully use your lib, when I'm more familiar with downshift

@techniq
Copy link
Owner

techniq commented Mar 6, 2018

@caub To keep the menu open, you could look at using downhift's stateReducer prop, which shows this very example.

I'd like to look into toggling, but also considered showing the selected items at the top of the list and it's makes more sense to see the selected items at the top upon opening the menu (or if the selections were set ahead of time, for instance from the query string). The trouble comes to not have the list jump when you put them at the top, and also not having the item "disappear" when you select it and it moves to the top (so you would kind of need both - the item added to the top without affecting what is shown, but also show the entry at the top for quick deselection (especially if you are paginating the items from the server).

Anyways, I look forward to your example, and thanks again :)

@caub
Copy link
Contributor Author

caub commented Mar 10, 2018

@techniq in my project I did it without downshift, with this way: https://codesandbox.io/s/yp4o52j6mx

@techniq
Copy link
Owner

techniq commented Mar 10, 2018

Throwing an error... looks like Dropdown is empty

image

@caub
Copy link
Contributor Author

caub commented Mar 10, 2018

it should work, tested on chrome/ff

@techniq
Copy link
Owner

techniq commented Mar 10, 2018

It works now, that was weird 😕 Looks nice 😄

Some initial feedback:

Setting a min-width on your <input /> will keep it from getting cut off (and flex-wrap'd sooner)

Before
image

After
image

I noticed arrow keys do not allow you to make selections

Downshift provides this (not to push you to use it, but might save you from implementing it yourself)

It would be nice to be able to press backspace with an empty input and remove items

See here for an example. It focuses the chip first, then removes it.

I've considered providing higher level components are part of miu-downshift (but still exposing MuiDownshift) to make it a little easier to use (especially for simple use cases, see #12 ). I might look into exposing a MultiSelect and possible MultiSelectGroup like you have, although even multi-select can be multi-faceted (chips, # selected with checkboxes, close after selection or not, etc) which is one reason I wanted to avoid it so far (I won't be able to support everyones use case, so hoped providing enough examples would at least give someone a way to copy/paste and iterate.

Anyways, sorry for the ramble. Looks good, and let me know if you need anything else from me. If you decide to use downshift and would want to PR something back to the project, I'd be most obliged.

@caub
Copy link
Contributor Author

caub commented Mar 10, 2018

Oh thanks, interesting

I've tried to add those left/right and delete selection in the first multi-select https://codesandbox.io/s/yp4o52j6mx it's

I'm sure it's possible to do it with Downshift, it basically replaces my Dropdown, it's quite equivalent, I was just not able to understand yet all the downshift flow, but yes, I'm sure you can be able to make those components. You'll need to be able to adjust downshift's width, I gave it a try, but not for long

Best, thanks

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.

2 participants