dojo / widgets

:rocket: Dojo - UI widgets.
https://widgets.dojo.io
Other
88 stars 66 forks source link

AccordionPane #14

Closed bitpshr closed 6 years ago

bitpshr commented 7 years ago

Enhancement

Add an AccordionPanel component

rishson commented 7 years ago

So we need to list out the initial features.

Things to consider:

For the initial implementation, my suggestion would be:

1 section open at once height stays constant - (overflow sections if needed) no disabled sections.

rishson commented 7 years ago

Also, I'd like to standardise on naming things that may have been called 'panel' in the past. I've been using semantic ui for too long, but i've got used to the brevity of their widget naming, so we could have:

accordion tab sideBar split

Thoughts?

msssk commented 7 years ago

I am also a fan of names being as long and descriptive as necessary, but no more. I think the "panel" suffix is often unnecessary.

For now, we should try to agree on the accordion name, and my vote goes to "accordion".

The overall naming scheme might be best tracked in a separate issue, but some quick thoughts:

rishson commented 7 years ago

tab is a contentious one. I've always viewed tab as both the tabStrip and the tabContents (for want of better names. tabPane and splitPane? (I just dislike 'panel' as its too retro).

rishson commented 7 years ago

Which would give us:

bitpshr commented 7 years ago

While I don't really care what suffix we use (e.g. "Pane" vs. "Panel" vs. anything else), I do think that every container component should use the same suffix. I think having one component that doesn't use it, like "Accordion", is confusing. This suffix sort of serves as an indicator that a component is a container-style component in my mind.

rishson commented 7 years ago

@bitpshr I've started to wonder if we need to consider all the widgets where the primary function is layout, such as :

To me, it seems clunky to have pane suffixes to these widgets. If the list was something like:

then I think its ok to drop the suffix at times. [shrug_emoji]

bitpshr commented 7 years ago

@rishson sounds good to me.

msssk commented 7 years ago

Some questions on what we want from an accordion pane:

bitpshr commented 7 years ago

I don't think an AccordionPane should accept arbitrary children. If another widget is needed next to the internal TitlePanes, the widget should just be a sibling to the AccordionPane within a wrapping div.

I like the idea of an exclusive option.

mwistrand commented 7 years ago

Based on the work already done for TitlePane, is a dedicated accordion widget even needed? It seems like an accordion is simply a collection of TitlePanes and any control logic can use existing properties:

v('div', {}, [
  w(TitlePane, {
    open: this.state.index === 0,
    onRequestOpen: () => this.setState({ index: 0 }),
    onRequestClose: () => this.setState({ index: undefined })
  }, [ /* ... */),

  w(TitlePane, {
    open: this.state.index === 1,
    onRequestOpen: () => this.setState({ index: 1 }),
    onRequestClose: () => this.setState({ index: undefined })
  }, [ /* ... */ ]),

  w(TitlePane, {
    open: this.state.index === 2,
    onRequestOpen: () => this.setState({ index: 2 }),
    onRequestClose: () => this.setState({ index: undefined })
  }, [ /* ... */ ])
]);
smhigley commented 7 years ago

I agree with @mwistrand, I don't really see a need for this widget. Would it add anything apart from limiting one open TitlePane at a time?

rishson commented 7 years ago

Accordion might be just a small wrapper widget to do just the limited opening. I think in this case, there is some justification in that it is a widget that you expect a toolkit to have, and if we have a tutorial explaining the above, rather than a widget, then I'm unsure what message that sends to the user base. On one hand we could look minimal, clean, elegant, or it could be perceived that we are asking too much of the user; that they have to do too much themselves..

smhigley commented 7 years ago

On the other hand, writing the above is already simpler that writing a Menu widget 😂.

rishson commented 7 years ago

I think we should maybe ask in dojo2 if small wrapped widgets are something we feel add value for users (Accordion, Loader)..

smhigley commented 7 years ago

Maybe it should be pushed down the road, at least? It doesn't seem to add a lot of value, especially compared to TimePicker or Tooltip or one of the others. That would also let us gauge against any feedback we (hopefully) get on beta2

rishson commented 7 years ago

but, but, its on my list! Won't someone think of the list!?

OK, maybe small widgets should be shelved until we have consensus on their inclusion.

sebilasse commented 7 years ago

Just wanted to mention that my Semantic UI dojo2 Integration also contains Accordion, posted a WIP preview here : https://github.com/dojo/widget-core/issues/559

eheasley commented 6 years ago

We are very near completion on this as a result of working on related tasks, so we will go ahead and get this taken care of for beta 4.