Skip to content

feat: add toolbar proposal#452

Closed
chpalac wants to merge 5 commits intoopenui:mainfrom
chpalac:feat/add-toolbar-proposal
Closed

feat: add toolbar proposal#452
chpalac wants to merge 5 commits intoopenui:mainfrom
chpalac:feat/add-toolbar-proposal

Conversation

@chpalac
Copy link

@chpalac chpalac commented Feb 2, 2022

Add Toolbar proposal

@chpalac chpalac mentioned this pull request Feb 2, 2022
33 tasks
@chrisdholt
Copy link
Collaborator

I see Fabric and Stardust both represented here, but I'm fairly sure that those are both represented now as Fluent UI. If I recall, this came up previously and we had looked at pointing at a single implementation (Fluent UI) rather than Fabric (no longer existing) and Stardust (part of Fluent UI merging as well...).

```html
<div>
<button />
<div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

arguably groupings of controls like this would be better as lists (give the menu element something to do) and not generic divs

<button />
<button />
</div>
<span />
Copy link
Collaborator

Choose a reason for hiding this comment

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

this span is here for?

- If the toolbar has a visible label, it is referenced by aria-labelledby on the toolbar element. Otherwise, the toolbar element has a label provided by aria-label.
- If the controls are arranged vertically, the toolbar element has aria-orientation set to vertical. The default orientation is horizontal.
- Implement focus management so the keyboard tab sequence includes one stop for the toolbar and arrow keys move focus among the controls in the toolbar.
- In horizontal toolbars, Left Arrow and Right Arrow navigate among controls. Up Arrow and Down Arrow can duplicate Left Arrow and Right Arrow, respectively, or can be reserved for operating controls, such as spin buttons that require vertical arrow keys to operate.
Copy link
Collaborator

Choose a reason for hiding this comment

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

i realize this is all a copy of the aria practices documentation, but I would submit there needs to be further investigation on expected arrow key behaviors here. Point being, the APG is demonstrative to how to implement ARIA features in accordance to the ARIA specification. Expected user behaviors for some of these patterns and their functionality need additional research.

#### Accessibility <a class="link" href="#accessibility" id="accessibility"></a>

- The element that serves as the toolbar container has role toolbar.
- If the toolbar has a visible label, it is referenced by aria-labelledby on the toolbar element. Otherwise, the toolbar element has a label provided by aria-label.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is how the ARIA practices is outlining how to use ARIA to provide a toolbar an accessible name. Could not this be more open to possibilities? for instance, rather than using aria-labelledby, if a fieldset were used as the base container with a role=toolbar, then it could leverage the legend element to specify a name for the toolbar.

the end goal of this could also be that HTML provides mechanisms to name certain elements. This topic has come up with HTML for use of naming landmark elements without use of ARIA, or using the title attribute. a toolbar would be another instance of where'd we need HTML to support this naming feature request.

Copy link
Collaborator

Choose a reason for hiding this comment

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

For a while now, I've wondered whether there was a way of adding <label for="something"> to elements other than <input>. Scott's comment leads me to think mine wasn't an isolated thought.

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeh, there's an issue on the HTML spec to do this... somewhere. i'd have to dig. but it's something that came up again recently per the recent request to add the <search> element.


- [Stardust](https://fluentsite.z22.web.core.windows.net/0.59.0/components/toolbar/definition)
- [Reakit](https://reakit.io/docs/toolbar/)
- [Radix UI](https://www.radix-ui.com/docs/primitives/components/toolbar)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have other prior art examples given, should we link out to those as well? Would we consider having more generic examples as well such as the toolbar from RoosterJS's rich text editor?

Comment on lines +36 to +44
<div>
<button />
<div>
<button />
<button />
</div>
<span />
<button />
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd probably prefer to understand the semantic representation of the structure here. Are the buttons here intended to be examples of slotted content? Similar to @scottaohara's question, what's the reason for the structure here? What are the named parts and why? In FAST, we identified a common pattern of having "start" and "end" content relating to layout, etc...that may not make sense here, but I think at the very least if there is additional DOM, understanding its intent is important.

Copy link
Collaborator

Choose a reason for hiding this comment

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

agreed. there a disconnect between what this snippet is meant to represent, particularly in comparison to what the accessibility requirements are saying are necessary, as well.

@gregwhitworth
Copy link
Member

Thanks for submitting this @chpalac - per the working mode can you open an issue about your interest in investigating this control/component: https://open-ui.org/working-mode

A key thing to denote within that issue is what your goal is for this component. Given that toolbar appears to generally overlap with other aspects I'd really like discourse on Open UI taking on this variation and you allowing your research to validate/invalidate the need for this component. Let me know if you have any questions

@gregwhitworth
Copy link
Member

@chpalac hey any update on this?

@gregwhitworth gregwhitworth marked this pull request as draft March 11, 2023 01:53
@gregwhitworth
Copy link
Member

This has been sitting for over a year so closing this out, it can always be re-opened if you want to take it further.

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.

5 participants