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

Block Switcher: Add "Transform into:" label at the top of the list #493

Closed
jasmussen opened this issue Apr 24, 2017 · 12 comments
Closed

Block Switcher: Add "Transform into:" label at the top of the list #493

jasmussen opened this issue Apr 24, 2017 · 12 comments
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later [Type] Enhancement A suggestion for improvement.

Comments

@jasmussen
Copy link
Contributor

jasmussen commented Apr 24, 2017

Say you have a text block. You click the switcher and see Heading and Quote (among others). You should also see Text there, but selected.

In this mockup, the block is already a quote, and the switcher button has just been clicked:

type switcher

@jasmussen jasmussen added the [Type] Enhancement A suggestion for improvement. label Apr 24, 2017
@youknowriad youknowriad added the Good First Issue An issue that's suitable for someone looking to contribute for the first time label Apr 24, 2017
@aduth
Copy link
Member

aduth commented Apr 24, 2017

You should also see Text there, but selected.

What does "selected" mean in this context? Dark background? Should the effect be removed from this selected option when hovering other options in the dropdown? And reapplied when mouseout from any of those other options?

@jasmussen
Copy link
Contributor Author

What does "selected" mean in this context? Dark background? Should the effect be removed from this selected option when hovering other options in the dropdown? And reapplied when mouseout from any of those other options?

Not sure if you're referring to the visual style, or the behavior, so I'll try and suggest thoughts on both.

Behaviorally, this would be similar to a select box. I.e. pick your country from a dropdown of countries, and your country still shows up both in the dropdown if you open it again.

Visually, let's try and go with the inverted dark gray version for selected, and blue icon/text on hovering non-selected items. Stylistically it is a bit heavy, I agree, but it reaches the AAA accessibility standards, so let's try and see if it also feels heavy in non-static use. If we still find it heavy after having used it for a bit, let's revisit. Other options include different colors, or a border as we use for the hover effect on formatting toggle buttons.

@ellatrix ellatrix added this to the Prototype Parity milestone Apr 26, 2017
@mtias mtias added the [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later label May 1, 2017
@mtias mtias removed this from the Prototype Parity milestone May 1, 2017
@jasmussen
Copy link
Contributor Author

Per discussion in #1249 (comment), it seems like maybe including the current block type in this list may not actually solve any usability issues.

Let's try a new approach. Let's embrace the fact that the current block doesn't show in the list. This isn't a dropdown. It's a switcher, and it lists block types you can transform into. Going a step further, let's label it. Perhaps like this:

screen shot 2017-06-21 at 14 45 45

@jasmussen jasmussen changed the title Block Switcher: Should list all transforms, including the selected Block Switcher: Add "Transform into:" label at the top of the list Jun 21, 2017
@androb
Copy link
Contributor

androb commented Oct 4, 2017

This was shipped in 1.3... although IMHO it opens a wider usability problem that I will create a separate issue for.

@androb androb closed this as completed Oct 4, 2017
@afercia
Copy link
Contributor

afercia commented Oct 12, 2017

@jasmussen noticed this while testing #2989

This "Transform into:" text should be tested a bit with screen readers, once things get a bit stable with these drop down menus. The technical reason is elements with an ARIA role="menu" should contain just menu items with a role="menuitem". The only other allowed thing within a menu is an element with role="separator" used to split items in groups. TL;DR reference: https://www.w3.org/TR/wai-aria-practices/#menu

I have no objections in keeping it if it doesn't break screen readers behavior 🙂
Screen reader users already have an equivalent information thanks to the aria-label "Change block type" on the toggle button.

From a visual perspective, have you considered to use a tooltip on the toggle button instead of a visible text within the menu? Something like:

screen shot 2017-10-12 at 17 25 13

@youknowriad
Copy link
Contributor

can we move the role=menu into a container of the menu elements only (adding a new div)

@afercia
Copy link
Contributor

afercia commented Oct 12, 2017

@youknowriad: DOM events don't work as expected with screen readers on normal <div> elements or other non natively interactive elements. VoiceOver has it own very peculiar interaction model. To my knowledge, all other major screen readers use "browse mode" and "forms mode". In the first mode they do intercept key presses. The browser is unaware a key press event occurred.
Only when a <div> has some special role, like "menu", screen readers understand they have to pass key presses back to the browser. In other words: the DOM events need to be attached to the element with role="menu" so I guess no, we can't do that.

@youknowriad
Copy link
Contributor

youknowriad commented Oct 12, 2017

@afercia I'm not following properly, you're saying the container should have a role=menu but not include the "Transform into:", that's exactly what I am proposing?

like in 5e5b8c0

@afercia
Copy link
Contributor

afercia commented Oct 12, 2017

@youknowriad ah that's different from what I thought. If the onKeyDown is still attached to the NavigableMenu and NavigableMenu has role=menu, that theoretically should work.
However, this should be tested. Please consider screen readers are not perfect tools. They expect a specific markup, specific roles, and specific DOM events, and interaction. The reason is very simple: they're designed based on the specifications. Every time we build something that deviates (even slightly deviates) from the specs, then there's a high chance screen readers won't work correctly.

For this reason, I'd still prefer the tooltip as in the screenshot above.

@jasmussen
Copy link
Contributor Author

Sorry for the late reply.

Aside from the discussion above (thank you for looking at solutions), I want to ask a question about an alternative route. In the most recent mockups in #2983, there's a "switcher" in the right most context menu, which shows a different design for items there — "Turn into Heading", etc. I can imagine two approaches from this:

  1. We change the switcher to have "Turn into" preceeding every item in the list, with icon and perhaps shortcut.

  2. We decide that the switcher in the quick toolbar is no longer necessary with the dropdown.

Do we like either of those solutions? Either of them have the "Transform into" text preceeding them. I personally like the idea of 1.

@afercia
Copy link
Contributor

afercia commented Oct 13, 2017

This one? nice :)

31435936-8e1f5a66-ae81-11e7-8a3e-b54fa48e8cf4

Yep then for consistency I'd rather try 1. I tend to think having labels always clearly identifying the available action is always preferable. There's a difference between a label that just says "Heading" and one that says "Turn into Heading": the latter always makes sense even when read out of context. So basically something like this?

screen shot 2017-10-13 at 09 50 36

On a side note: some block names should maybe be improved a bit. In my language, "Preformatted" doesn't make much sense :) it's an adjective, not a noun.

Trasforma in Heading
Trasforma in Citazione
Trasforma in Lista
Trasforma in Preformattato (?)
Trasforma in Versetto ( 🙈 ) 

@jasmussen
Copy link
Contributor Author

So basically something like this?

Yes, basically, except weith a few visual tweaks to style, padding, and such.

On a side note: some block names should maybe be improved a bit. In my language, "Preformatted" doesn't make much sense :) it's an adjective, not a noun.

I kind of want to not include either "Verse" or "Preformatted" in that menu ... I know, this is controversial, and it's not a mountain I will die on. But they feel... confusing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

8 participants