1733662 - Attachment arrow key navigation is broken in various ways

Status: NEW

(Thunderbird :: Message Compose Window, defect, P2)

Description

[Henry Wilkes (they/them) [:henry-x]]

There are a few problems with the arrow key navigation in the composition window (handled here https://searchfox.org/comm-central/rev/534a966e11fc09f05ff11aba4bb3e65e881ae22d/mail/components/compose/content/MsgComposeCommands.js#7518):

1. The horizontal arrow keys are the wrong way around for right-to-left locales.
2. The vertical arrow keys assume each row has the same number of items as on the first row (it uses https://searchfox.org/comm-central/rev/534a966e11fc09f05ff11aba4bb3e65e881ae22d/mail/base/content/widgets/mailWidgets.js#1822), which is not necessarily true if some attachments have longer names than others. This means that pressing up and down can make you jump several rows, or keep you on the same row.
3. The aria-orientation is vertical (default for a role=listbox element) even though Left-Right keys are used to go one step forward or backward.
4. If the bucket is overflowing vertically, arrow key navigation does not properly scroll to the item that was moved to.

Moreover, it is inconsistent with the message pane #attachmentList. That attachment-list uses the default richlistbox navigation keys, which is Up-Down and matches the aria-orientation=vertical. It does have the same problem as point 4 though.

Solutions

Note that since the attachment-lists uses flex-wrap: wrap, the items are not placed in a single line, but neither are they placed in a grid.

Probably for both attachment-lists, we should set the aria-orientation to horizontal to match the flex-direction and use Left-Right to iterate by one item.

I'm not as sure what the response to Up-Down should be, or whether we should completely ignore it. For example, what should we do in the following situations?

Example 1

attachemnt2.txt selected and press Down:

[attachment1.txt]  <attachment2.txt>  [attachment3.txt]
[attachment-name-4.txt]  [long-attachment-name-5.txt]

Example 2

long-attachment-name-5.txt selected and press Up:

[attachment1.txt]  [attachment2.txt]  [attachment3.txt]
[attachment-name-4.txt]  <long-attachment-name-5.txt>

Example 3

very-very-long-attachment-name2.txt selected and press Down:

[attachment1.txt]  <very-very-long-attachment-name2.txt>  [attachment3.txt]
[attachment-name-4.txt]  [short5.txt]  [attachment6.txt]

I'm also unsure if the vertical navigation might be confusing to screen reader users since the vertical alignment of the items will not be know. So pressing Up-Down will produce potentially inconsitent jumps. The aria-orientation=horizontal should hopefully hint to use Left-Right for navigation, but I'm not sure how good screen readers are at passing on the info.

Comment 1

[Henry Wilkes (they/them) [:henry-x]]

From https://phabricator.services.mozilla.com/D127288#4157451

The #attachmentList appears gridded, even though it is flex-wrapped like #attachmentBucket, because of calls to setOptimumWidth https://searchfox.org/comm-central/rev/780f009ce394fe1cfca1c8ab07ddfccc4b3e09bd/mail/base/content/widgets/mailWidgets.js#1865 which sets each child to have the same width.

Note that if we wanted to do this for #attachmentBucket, which is dynamic unlike #attachmentList, we would have to recalculate this every time an attachment is added, removed or renamed (and some other cases).

Moreover, there is a bug in the usage of setOptimumWidth:

1. Shrink the message pane horizontally and select an email with an attachment whose name is longer than the horizontal width.
2. Open the attachment area. The attachment item should have overflowed with an ellipsis.
3. Grow the message pane horizontally.

In the last step, the attachment item doesn't grow to the fit the new width. This is because the method uses getBoundingClientRect().width, which can be shorter than the preferred width. And the method is not refreshed when there is more horizontal space.

Comment 3

[Alessandro Castellani [:aleca]]

A couple of possible solutions we might explore are:

1. Implement the setOptimumWidth method and deal with all the possible interactions.
2. Convert the #attachmentBucket into a grid area and use grid-template-columns: repeat(auto-fill, minmax(300px, 1fr));, with that 300px something we calculate as optimal size for each column. Also this calculation would need to happen in various scenarios, but at least we can only add an inline style to the whole bucket instead of each item.

I would lean more towards option 2.
Any other possible approach?

I'm also exploring some UI solutions to allow users to decide where to show the attachment bucket, but that will need more time and the alignment of attachments should be fixed nonetheless.

Comment 4

[Henry Wilkes (they/them) [:henry-x]]

(In reply to Alessandro Castellani [:aleca] from comment #3)

A couple of possible solutions we might explore are:

1. Implement the setOptimumWidth method and deal with all the possible interactions.
2. Convert the #attachmentBucket into a grid area and use grid-template-columns: repeat(auto-fill, minmax(300px, 1fr));, with that 300px something we calculate as optimal size for each column. Also this calculation would need to happen in various scenarios, but at least we can only add an inline style to the whole bucket instead of each item.

I looked into this before and the problem with either approach is that they would make the items equal width. E.g. with two attachments where one is longer than the other you get the layout

[short one                    ][loooooooooooooooooooooong one]

rather than

[short one][loooooooooooooooooooooong one]

I think if we want nice behaviour where long attachments are given space if it is available, but we don't want flex-wrapping, we could either:

1. keep it simple and just have a basic list with no wrapping (each item is on its own line), or
2. we use a grid with a custom layout algorithm because repeat(auto-fit, ...) is too limited for our needs (I may be wrong, but I haven't been able to get it to work well).

Comment 5

[Alessandro Castellani [:aleca]]

After further discussion, here's the proposed implementation plan:

1. Offer 3 display options for the attachments:
   • Auto-fill columns (default, with an arbitrary size for the max-width of each column).
   • Compact view (the current).
   • Single column.
2. These options will be available in the context menu of the attachment panel, in the View > Attachments menubar, and in the Attach toolbar button dropdown menu.
3. The chosen display option will be made persistent through the Services.xulStore.

Comment 6

[Danny Colin [:sdk]]

First of all, I get that not everyone is happy with the changes and I don't want to minimize their use case. However, adding tons of options isn't IMO a better solution. It gets quickly confusing to navigates all the options for a lot of users. I mean the menubar is at this point a giant menu of stuff where it's hard to find what you really need. Further, there's "options" buttons everywhere for everything and that also adds a lot of UX complexity to the UI.

(Maybe this should be discussed in more depth somewhere else).

Comment 7

[Alessandro Castellani [:aleca]]

I hear you, and thanks for the feedback.

However, adding tons of options isn't IMO a better solution. It gets quickly confusing to navigates all the options for a lot of users.

These options will be only for those users that need it, and the "complexity" in this case is very very minimal as the 3 display views are achievable with a CSS one liner.
The only real "complexity" is adding the menubar and context menu items, but those are necessary to make it discoverable to all users, no matter their workflow or entry point.

Users that don't need to change anything of the default display won't even see the new options and won't deal with the "confusion".

Comment 8

[Magnus Melin [:mkmelin]]

I agree with Danny, adding options how to display is really just dodging finding a solution that works for all. The problem isn't really "I want to configure this to look another way", but "I want it to work better, by default". Then of course, the bug is about keyboard navigation which is ultimately not helped by adding more options: you just add 3 cases that all need to work well, instead of one. (Side note: we had view options for these for years, it was broken since many releases and nobody had noticed, so probably people didn't care. IIRC Henry removed the last remaints of that sometime in the last 6mo).

Personally, for the look, I would get away from the griddy look we have, and just let the attachments have their natural width (up to some limit?). It would be good if they were more pill-like so that one sees better what the attachment is (connect name+size).

Comment 9

[Henry Wilkes (they/them) [:henry-x]]

(In reply to Magnus Melin [:mkmelin] from comment #8)

Then of course, the bug is about keyboard navigation which is ultimately not helped by adding more options: you just add 3 cases that all need to work well, instead of one.

Regarding keyboard navigation: I think I will just use one-directional keyboard navigation. I'm leaning towards Up-Down since this is what we do for the 3pane attachment list and other richlistboxes. I don't think this area needs 2-dimensional keyboard navigation (which would normally be for role=grid, which would be more complicated).

(Side note: we had view options for these for years, it was broken since many releases and nobody had noticed, so probably people didn't care. IIRC Henry removed the last remaints of that sometime in the last 6mo).

Yes, this was https://hg.mozilla.org/comm-central/rev/a9015ae2a270 . We used to have tile view and large view, but this was for the individual attachment items, rather than the attachment list layout. Moreover, it was a hidden preference not exposed in the UI. But I get the point about options being unused and degrading.

Personally, for the look, I would get away from the griddy look we have, and just let the attachments have their natural width (up to some limit?)

The 3pane attachment list uses a grid-like display, the compose window uses a flex-wrapping display (which I think is what you want). It is the latter that users were unhappy with. Some users want a vertical list, which is easier to scan-read.

If we're worried about lots of options, we could just limit it to the two most important ones (and easiest to implement):

• Compact (flex-wrap)
• Single column (vertical list)

It would just be two CSS layouts:

.compact {
  display: flex;
  flex-direction: horizontal;
  flex-wrap: wrap;
}

.single-column {
  display: flex;
  flex-direction: vertical;
}

Alternatively, if @aleca finds a nice way to display the attachment list as a column on the right hand side of the editor, we could only use .compact for the horizontal attachment bar, and .single-column for the vertical attachment bar.

In any case, I'm going to wait until after bug 1741361 before I do anything here since it'll help clean up this area. And, if we want to make similar changes to the 3pane attachment list, I might delay this until after bug 1732260 as well.

Comment 10

[Thomas D. (:thomas8)]

(In reply to Danny Colin [:sdk] from comment #6)

First of all, I get that not everyone is happy with the changes and I don't want to minimize their use case.

Serving real life use cases is of utmost importance. Users are unhappy because their real-life work flows are now harder and less efficient than before - interwoven tetrix-bricks are harder to parse than vertically aligned items.

However, adding tons of options isn't IMO a better solution.

Let's refrain from exaggerations. 3 layout options for an important spot in the UI isn't "tons of options".

It gets quickly confusing to navigates all the options for a lot of users.

Alex is an expert for making them available nicely-worded in context (exactly where needed), and they'll be tucked away under a single small contextual UI element plus the menus. I don't see much room for confusion here. If you don't need them, just ignore them, and they won't get in your way.

I mean the menubar is at this point a giant menu of stuff where it's hard to find what you really need.

Another commonplace, and I disagree: Our menus are extremely well sorted for those who actually need them. Offering every command and every option also in the main menus is actually helpful for accessibility and exploring things in a systematic way.

Further, there's "options" buttons everywhere for everything and that also adds a lot of UX complexity to the UI.

Sorry, but you can't have your cake and eat it - contextual options are exactly for you and everyone to avoid the going into vast main menus. I don't think that having a couple of dots dotted across the UI is adding much complexity. It's an excellent compromise between keeping the main UI clean but still having options at your fingertips in the context where they apply.

(Maybe this should be discussed in more depth somewhere else).

TL;DR: Different users have different needs. Catering for many different needs is a key reason why Thunderbird is successful. UX-efficiency is part of our brand core. Sometimes one-for-all may work, but mostly it won't. If I wanted one-for-all (with very little flexibility), I could just use webmail...

Comment 11

[Thomas D. (:thomas8)]

(In reply to Alessandro Castellani [:aleca] from comment #5)

After further discussion, here's the proposed implementation plan:

1. Offer 3 display options for the attachments:
   • Auto-fill columns (default, with an arbitrary size for the max-width of each column).
   • Compact view (the current).
   • Single column.

Awesome, LGTM!

Auto-fill sounds a bit strange to me. Do you mean auto-fit? Or maybe replace Auto-fill columns with a simple Automatic columns - then all three options are noun phrases, and easier to translate.
Maybe in this order? Which keeps the two column options together:

• Automatic columns (default)
• Single column
• Compact view

Or by density?

• Compact view [most dense]
• Automatic columns [default, medium density]
• Single column [low density]

I guess all three ways of ordering these options have advantages.

2. These options will be available in the context menu of the attachment panel, in the View > Attachments menubar, and in the Attach toolbar button dropdown menu.

Perfect.

3. The chosen display option will be made persistent through the Services.xulStore.

Ah well... ;-)

Comment 12

[Thomas D. (:thomas8)]

(In reply to Henry Wilkes [:henry] from comment #9)

Regarding keyboard navigation: I think I will just use one-directional keyboard navigation. I'm leaning towards Up-Down since this is what we do for the 3pane attachment list and other richlistboxes. I don't think this area needs 2-dimensional keyboard navigation (which would normally be for role=grid, which would be more complicated).

Hmmm. Is this not some custom element which we want to use in more spots? Perhaps if we get 2-dimensional keyboard navigation right in the custom element (once-only effort), we could benefit everywhere? Such navigation functionality should really be inbuilt.
Being limited to navigation which differs from the actual direction I want to take might be a bit irritating for the multi-layouts (vs. single-column).
But yeah, if we'd limit to 1 dimension, it should probably be up-down.

Comment 13

[Henry Wilkes (they/them) [:henry-x]]

(In reply to Thomas D. (:thomas8) from comment #12)

Hmmm. Is this not some custom element which we want to use in more spots? Perhaps if we get 2-dimensional keyboard navigation right in the custom element (once-only effort), we could benefit everywhere? Such navigation functionality should really be inbuilt.

To put my reasoning in more detail:

• The most simple way to conceptualise the attachment data is as a role=listbox. A grid concept is just more complex without any benefit. This implies one-dimensional navigation is going to be implied by the accessibility tree.
• The xul:richlistbox only implements keyboard controls in the vertical direction, which matches the default aria-orientation of a listbox.
• The 3pane already uses this vertical navigation, even though it is gridded.
• 2-dimensional navigation doesn't work well with flex-wrapped lines (see comment 0).

So whilst primarily-visual keyboard users in the grid display might like to be able to move in 2-dimensions, I don't think it would be worth it (note we would have to implement multi-selection controls as well for the extra direction).

Being limited to navigation which differs from the actual direction I want to take might be a bit irritating for the multi-layouts (vs. single-column).

What I would like to do is be able to change the orientation of the controls for the compact and gridded display so they are Right-Left and have a matching aria-orientation. However, I think I'd rather modify our custom html listbox to take the aria-orientation into account, and use this for the attachment list instead of the xul:richlistbox. I'll add this to the "to do" list for bug 1751978. But I think Up-Down work ok for the time being.

Comment 14

[Henry Wilkes (they/them) [:henry-x]]

Overall, I think this should wait until bug 1752532 to use the listbox widget and one-dimensional keyboard navigation. The direction of navigation should match the layout direction. For example, as it is now (with flex-wrapping) we should use the "horizontal" navigation. But if we implement other views "vertical" navigation would work fine. E.g. the "single column" layout in comment 5. I explicitly constructed the SelectionWidgetController to allow for this dynamic change in direction.