[RFC] Toolbar Component #461

joheredi · 2018-11-09T21:45:32Z

Feature Request

Overview

The toolbar is a component which houses different tools/buttons. The toolbar can have multiple groups to help organize buttons.

Requirements

Button Groups
Button Overflow Flyout
Sub Menus
Custom Picker/Popup

Mock: Toolbar with submenu

Mock: Toolbar with Custom Picker

Mock: Button overflow flyout:

Proposed solution

Use the following stardust components to build up the Toolbar:

Menu + MenuItem - For the buttons
For Submenu/Dropdown we could resume the work on Add possibility to render submenu in MenuItem #126:
- SubMenu
- We can also leverage this Submenu for the overflow flyout
Popup - For custom picker
Grid to layout the Toolbar

interface IToolbarItem {
  name: string;
  /** Type of command item */
  itemType: "button" | "picker";
  /** Primary content - i.e. Button Text*/
  content?: string;
  /** Icon name or shorthand */
  icon?: string | IconShorthand;
  /** Custom picker content to render within the popup */
  pickerContent?: JSX.Element;
  /** Dropdown Items */
  subMenu?: MenuItemShorthand[];
}

interface IToolbarGroup {
  name: string;
  /** Items to display in the group*/
  groupItems: IToolbarItem[];
  /** Items to showup in the overflow flyout*/
  overflowItems: IToolbarItem[];
  /** Group display order override, if set takes precedence over groups without default order */
  order?: number;
}

Internal structure:

The main container would be a Grid
Each column in the grid would host a ToolbarGroup
Each ToolbarGroup would be a Menu

Toolbar

render = () => {
  return <Grid content={this.props.groups} />;
};

ToolbarGroup

  render = () => {
    return <Menu
      items={this.props.items}
      renderItem={this.renderItem} />
  }

  renderItem = (_menuItem, { itemType, pickerContent, ...props}) => {
    /**
     * This will decide whether to render a simple MenuItem or Popup
    */
    switch (itemType){
      case "button":
        return <MenuItem {...props} />
      case "popup":
        return <Popup trigger={<MenuItem {...props}}  content={pickerContent} />}
    }
  }

Usage example

<Toolbar groups={[basicFormatting, extraFormatting]} />;

/** Group with only Buttons */
const basicFormatting: IToolbarGroup = {
  name: "BasicFormatting",
  groupItems: [
    { name: "bold", itemType: "button", icon: "bold", onClick: () => console.log("triggered Bold command") },
    { name: "italic", itemType: "button", icon: "italic", onClick: () => console.log("triggered Italic command") },
    { name: "underline", itemType: "button", icon: "underline", onClick: () => console.log("triggered Underline command") }
  ],
  overFlowItems: [
    { name: "subscript", itemType: "button", icon: "subscript", onClick: () => console.log("triggered Subscript command") },
    { name: "superscript", itemType: "button", icon: "superscript", onClick: () => console.log("triggered Superscript command")}
  ]
};

/** Group with a dropdown and a custom picker */
const extraFormatting: IToolbarGroup = {
  name: "ExtraFormatting",
  groupItems: [
    { name: "heading", itemType: "button", submenu: headingSubmenuItems },
    { name: "hyperlink", itemType: "picker", pickerContent: pickerContent }
  ]
};

/** Items in the Submenu  */
const headingSubmenuItems: MenuItemShorthand[] = [
  { key: "Paragraph", content: "Paragraph", onClick: () => console.log("triggered Paragraph command") },
  { key: "Heading1", content: "Heading 1", onClick: () => console.log("triggered Heading1 command") },
  { key: "Monospaced", content: "Heading 2", onClick: () => console.log("triggered Monospaced command") }
];

/** Custom component to be rendered within the Picker, this can be any react component*/
const pickerContent = (
  <div>
    /** ... */
    <button onClick={() => console.log("triggered SomeCommand command")}>
      Some command
    </button>
  </div>
);

The text was updated successfully, but these errors were encountered:

WrathOfZombies · 2018-11-09T23:03:37Z

Looking at the patterns in Stardust, I think it would make more sense to have something like this:

<Toolbar>
  <Toolbar.Group name="BasicFormatting">
    <Button name="bold" icon="underline" onClick={() => {}} />
  </Toolbar.Group>
  <Toolbar.Group name="ExtraFormatting">
    <Picker name="picker">
      {pickerContent}
    </Picker>
  </Toolbar.Group>
</Toolbar>;

WrathOfZombies · 2018-11-09T23:06:26Z

We may also want to add support for alignment. I think a good example can be something like Fabric's commandbar or an extension to Semantic UI React - Menu component

stuartlong · 2018-11-09T23:21:04Z

The Picker / subMenu parts seem a little feature specific. Does the toolbar need to care if a given button is rendering a Picker or not?

joheredi · 2018-11-09T23:39:39Z

@WrathOfZombies it seems that ShorthandApi is generally preferred over ChildrenApi. Here are a few advantages of Shorthand over ChildrenApi https://stardust-ui.github.io/react/shorthand-props also in #79 @levithomason put together a nice analysis on supporting ChildrenApi

I like the idea of supporting alignment. I'll consider that!

hnager · 2018-11-11T00:41:58Z

@stuartlong from an accessibility standpoint it may be important to identify that a given button will render a popup when pressed. The submenu parts may not need to be here, but there should be a way to assign accessibility traits/roles to specific buttons in the toolbar.

sophieH29 · 2018-11-14T11:44:44Z

@joheredi Toolbar component it's a nice idea, but I don't feel like it should be a part of Stardust components.
Have you had a chance to look at Menu example in docs, called "Menu as a Toolbar"? By setting toolbarBehavior for a Menu component, it sets correct aria-* attribute, and the items of the Menu can be whenever you want, either popup or drop-down.

 <Menu
        items={items}
        iconOnly
        accessibility={ToolbarBehavior}
        aria-label="Compose Editor"
      />

  const items = [
     ...
     {
      key: 'cloud',
      icon: {
        name: 'cloud',
        circular: true,
        xSpacing: 'both',
        size: 'small',
      },
     accessibility: ToolbarButtonBehavior,
      'aria-label': 'Cloud Tool',
    },
    ...
   ]

Recently, I prototyped the similar popup picker which contains Grid of images and opens from clicking on a button. Please, reach me, if you want to see more details on it and we can also try to prototype the similar toolbar that will match your needs.

jurokapsiar · 2018-11-20T12:24:15Z

We had several discussions around this. It looks like Menu component provides most of the functionality already. Things that are missing from menu right now are:

divider (@miroslavstastny can you post a link to the RFC?)
submenu (see PRs feat(menuItem: add menu prop) #357 and feat: Updating buttons to be in line with new styles #216 which have been closed without completion)

Submenu would allow creating the overflow menu.

If the Paragraph button in the mock above would be implemented as a submenu, it would be easy to dynamically move it to the overflow menu and have a second level submenu out of it.

layershifter · 2018-11-22T10:00:47Z

I will agree there with @sophieH29 and @jurokapsiar, we need this component for Teams, but it's a very specific thing and doesn't fit Stardust. I'm fully agree that it can be built with the Menu component and we just need to add missing functionality there.

jurokapsiar · 2018-11-22T15:19:09Z

To recap submenu requirements:
Submenu is already available in #126 and #357, the latter looks more advanced. It needs some consolidation, finishing and adding keyboard handlers.

Once submenu functionality gets added to the Menu component, we can use that for both the Formatting 'dropdown' as well as for the overflow.

The overflow is at that point out of scope of Stardust and the consumer can implement that on his side using submenu.

In general, toolbar needs to follow https://www.w3.org/TR/wai-aria-practices/examples/toolbar/toolbar.html - it should be possible to recreate that using our Menu component and Toolbar/ToolbarButton behaviors. The difference will be that Menu renders <ul> and <li> instead of <div>s that are used in the aria practices, but I think screen readers should be able to handle that (needs to be tested after implementation)

jurokapsiar · 2019-01-02T13:03:43Z

Submenu is now available.
On top of it, following issues will need to be implemented to allow toolbar functionality as discussed above:
#667
#669

bmdalex · 2019-03-27T19:15:06Z

Reasons why Toolbar would be useful after all (maybe I'm repeating myself, I just skimmed through comments):

1. Ability to have multiple items in active state

This behavior cannot be easily achieved using the Menu component because the active prop on MenuItem is exclusive (only one item can be active).

2. Ability to customize `active` prop on an item to act like a toggle (sticky items) or introduced the concept of `pressed`

3. Dedicated accessibility functionality integrated in the component

Right now, in order to achieve the accessibility behavior of a Toolbar we need to use a Menu component and add behaviors to the Menu component and it's items, which can get quite verbose:

<Menu
  items={[
    {
      accessibility: toolbarButtonBehavior,
      content: '..',
    },
    {
      accessibility: toolbarButtonBehavior,
      content: '..',
    },
  ]}
  accessibility={toolbarBehavior}
/>

With dedicate Toolbar we could just do:

<Toolbar
  items={[
    { content: '..' },
    { content: '..' },
  ]}
/>

4. Ability to use correct markup (div > button instead of ul>li ) as recommended by W3C

This is not critical but it's recommended by W3C: https://www.w3.org/TR/wai-aria-practices-1.1/examples/toolbar/toolbar.html

5. Other components implement it so we can argue it's not a one-off thing

For this we could definitely reuse logic and styles from Menu component, which would make the Toolbar component quite thin. Thoughts?

@jurokapsiar
@kuzhelov
@layershifter
@sophieH29

bmdalex · 2019-03-27T19:16:48Z

also requested in #1007

jurokapsiar · 2019-04-01T20:41:33Z

I think 1 and 2 can be rephrased and both are valid requirements for both Menu and Toolbar:

make active Menu state optional (only small subset of menus need to keep track of their active item)
introduce concept of pressed for Menu/Toolbar item

3 will be solved by #1008
For 4, the markup is correct with Menu component. The only difference is that the wrapper is rendered always, not only for buttons with submenu.

I think we can either introduce a <Toolbar /> component or a <Menu toolbar /> variant for better readability, but agree with the code reuse. Most of the requirements mentioned by @Bugaa92 are valid for both Menu and Toolbar.

bmdalex · 2019-04-02T10:28:08Z

@layershifter
@mnajdova
@kuzhelov
@miroslavstastny
@alinais

Should we vote on this one?

From @jurokapsiar 's last reply it looks like we could achieve most of the scenarios with the current Menu component. Do we want to do that or will Menu become too complex?

KyleBastien · 2019-04-04T00:48:20Z

One requirement that I don't think I've seen in the comments here yet (could be wrong..), that I would love to see added...

Some way to control positioning of certain items in the Menu.

Going back to Fabric's CommandBar, one way this is expressed is as farItems. I feel like this is a very common use case, that some items in a menu should be on the left, while others are visually separated to the right.

In the current "Icon and content" example for the Menu, it would probably be preferred for the "Search" icon/behavior to live on the far right side of that Menu. Obviously this would all be reversed in RTL.

bmdalex · 2019-04-30T22:01:26Z

as per discussions we had recently with @levithomason @kuzhelov and @miroslavstastny we will proceed with a new Toolbar component; first API version soon

kuzhelov · 2019-06-07T12:41:00Z

addressed by #1408 with base implementation for Toolbar component. If there are some features missed, please, consider to file a separate issue (one per feature), with the description that would explain/justify the feature.

pkumarie2011 added the vsts Paired with ticket in vsts label Nov 12, 2018

kuzhelov closed this as completed Jun 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC] Toolbar Component #461

[RFC] Toolbar Component #461

joheredi commented Nov 9, 2018

WrathOfZombies commented Nov 9, 2018

WrathOfZombies commented Nov 9, 2018

stuartlong commented Nov 9, 2018

joheredi commented Nov 9, 2018

hnager commented Nov 11, 2018

sophieH29 commented Nov 14, 2018 •

edited

Loading

jurokapsiar commented Nov 20, 2018

layershifter commented Nov 22, 2018

jurokapsiar commented Nov 22, 2018 •

edited

Loading

jurokapsiar commented Jan 2, 2019

bmdalex commented Mar 27, 2019 •

edited

Loading

bmdalex commented Mar 27, 2019

jurokapsiar commented Apr 1, 2019

bmdalex commented Apr 2, 2019

KyleBastien commented Apr 4, 2019 •

edited

Loading

bmdalex commented Apr 30, 2019

kuzhelov commented Jun 7, 2019

[RFC] Toolbar Component #461

[RFC] Toolbar Component #461

Comments

joheredi commented Nov 9, 2018

Feature Request

Overview

Requirements

Proposed solution

Internal structure:

Usage example

WrathOfZombies commented Nov 9, 2018

WrathOfZombies commented Nov 9, 2018

stuartlong commented Nov 9, 2018

joheredi commented Nov 9, 2018

hnager commented Nov 11, 2018

sophieH29 commented Nov 14, 2018 • edited Loading

jurokapsiar commented Nov 20, 2018

layershifter commented Nov 22, 2018

jurokapsiar commented Nov 22, 2018 • edited Loading

jurokapsiar commented Jan 2, 2019

bmdalex commented Mar 27, 2019 • edited Loading

1. Ability to have multiple items in active state

2. Ability to customize active prop on an item to act like a toggle (sticky items) or introduced the concept of pressed

3. Dedicated accessibility functionality integrated in the component

4. Ability to use correct markup (div > button instead of ul>li ) as recommended by W3C

5. Other components implement it so we can argue it's not a one-off thing

bmdalex commented Mar 27, 2019

jurokapsiar commented Apr 1, 2019

bmdalex commented Apr 2, 2019

KyleBastien commented Apr 4, 2019 • edited Loading

bmdalex commented Apr 30, 2019

kuzhelov commented Jun 7, 2019

sophieH29 commented Nov 14, 2018 •

edited

Loading

jurokapsiar commented Nov 22, 2018 •

edited

Loading

bmdalex commented Mar 27, 2019 •

edited

Loading

2. Ability to customize `active` prop on an item to act like a toggle (sticky items) or introduced the concept of `pressed`

KyleBastien commented Apr 4, 2019 •

edited

Loading