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

[TransferList] Add new component #15232

Merged
merged 16 commits into from Apr 9, 2019

Conversation

mbrookes
Copy link
Member

@mbrookes mbrookes commented Apr 6, 2019

@mui-pr-bot
Copy link

mui-pr-bot commented Apr 6, 2019

No bundle size changes comparing f1e2e55...4a32c2f

Generated by 🚫 dangerJS against 4a32c2f

@mbrookes mbrookes added docs Improvements or additions to the documentation low priority labels Apr 6, 2019
@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 7, 2019

What do you think of

  • Having only two buttons in the middle but a checkbox at the top like Ant to select everything? I find it more intuitive.
  • Creating a new documentation page for this component demo, like https://ant.design/components/transfer/?

@joshwooding
Copy link
Member

Maybe creating a new section to differentiate it from base components as well?

@mbrookes
Copy link
Member Author

mbrookes commented Apr 7, 2019

Having only two buttons in the middle

You can have only two buttons if you like – it's addressed in the demo description. Why not provide useful code that can be omitted, rather than omitting it and it having to be recreated if required?

I don't believe the list component has the concept of a "select-all" checkbox. The double chevron is a common pattern; also, one click vs. two.

Creating a new documentation page for this component demo

Then it becomes a component in its own right, needing tests and types. It's a simple, well structured demo (compared to some of the monoliths we have adopted), and is directly related to lists. If someone wants to run with it and give it a bunch of features, be my guest.

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 7, 2019

@mbrookes I'm not sure we are on the same page:

  • The transfer component involves different core components. The link with a list component is far fetched. They are not used in the same context. I think that a dedicated /demos/transfer documentation page will be better. It doesn't mean we have to export something from the core. We don't with the autocomplete. The other advantage is that we can simply track the documentation page traffic.
  • I'm confused by all these buttons in the middle, I'm probably not alone. I believe we could make it simpler:
    Capture d’écran 2019-04-07 à 14 57 50

https://element.eleme.io/#/en-US/component/transfer#transfer

Capture d’écran 2019-04-07 à 14 58 48

https://ant.design/components/transfer/#header

Capture d’écran 2019-04-07 à 15 00 37

http://uxco.re/components/transfer/

I can find very little component to benchmark against 😔.

@eps1lon
Copy link
Member

eps1lon commented Apr 7, 2019

You can have only two buttons if you like – it's addressed in the demo description. Why not provide useful code that can be omitted, rather than omitting it and it having to be recreated if required?

I would support this philosophy. The alternate implementations do not have the functionality of transfering all items.

@mbrookes
Copy link
Member Author

mbrookes commented Apr 7, 2019

@oliviertassinari

The transfer component involves different core components.

List and Button (and Checkbox). It's a slightly more advanced list example. But if the goal is to boost the perceived number of components supported, then sure, it could have its own page.

It doesn't mean we have to export something from the core. We don't with the autocomplete.

Fair point.

@eps1lon

The alternate implementations do not have the functionality of transfering all items.

You can in a couple of @oliviertassinari's examples by using the "select-all" checkbox before transferring.

Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

Nice start! :)

docs/src/pages/demos/transfer-list/TransferList.js Outdated Show resolved Hide resolved
docs/src/pages/demos/transfer-list/TransferList.js Outdated Show resolved Hide resolved
docs/src/pages/demos/transfer-list/TransferList.js Outdated Show resolved Hide resolved
oliviertassinari and others added 2 commits April 8, 2019 00:39
Co-Authored-By: mbrookes <github@nospam.33m.co>
Co-Authored-By: mbrookes <github@nospam.33m.co>
@mbrookes
Copy link
Member Author

mbrookes commented Apr 8, 2019

Here are a few more (some dated) examples. (I also found the ones you found. 🍒:wink: ). Most use the double chevron for move all, though a couple don't offer any move-all feature.
None has the "swap selected" button, so I've removed it.

https://github.com/RickStrahl/vue-mover

http://uxco.re/components/transfer/

http://otndnld.oracle.co.jp/document/products/as10g/101300/B25221_03/web.1013/b25386/web_complex008.htm

https://codinginthetrenches.com/2014/05/24/create-an-object-based-dual-list-shuttle-with-knockoutjs/

https://docs.oracle.com/cd/E29049_01/web.1112/e16182/lists.htm
(Figure 30-7 SelectManyShuttle Component)

https://xdsoft.net/jquery-plugins/shuttle/

So let's do a demo for both move-all and select-all 😄 :

image

I also came across the wikipedia definition:

https://en.wikipedia.org/wiki/List_builder

@mbrookes mbrookes force-pushed the docs-example-transfer-list branch 2 times, most recently from 4379a2c to ff0633b Compare April 8, 2019 00:13
Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

I love it :)

Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

I was too lazy to report it. Nice fix!

@oliviertassinari oliviertassinari changed the title [docs] Add transfer-list example [TransferList] Add new component Apr 8, 2019
@oliviertassinari oliviertassinari added new feature New feature or request and removed low priority labels Apr 8, 2019
@mbrookes
Copy link
Member Author

mbrookes commented Apr 8, 2019

Okay, done tweaking. :)

@oliviertassinari oliviertassinari merged commit 2426de7 into mui:next Apr 9, 2019
@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 9, 2019

We need a good framework for finding the components that are the most requested by our users. This transfer list component is not a highly requested one, but it's always nice to have it :). We can always defer it somewhere else in the future if it's slowing us down and if relatively little people use it.

@mbrookes well done :)

@mbrookes mbrookes deleted the docs-example-transfer-list branch March 15, 2020 23:02
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 new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants