Draft: Add jump to date functionality to date headers in timeline #7317

MadLittleMods · 2021-12-09T09:40:39Z

Add jump to date functionality to date headers in timeline

Part of MSC3030: matrix-org/matrix-spec-proposals#3030

Experimental Synapse implementation added in matrix-org/synapse#9445

Dev notes

Combining React callback refs and ref-refs,

const attachRef = (input) => {
    this.input = input;

    if (typeof this.props.inputRef === 'function') {
        this.props.inputRef(input);
    } else if(this.props.inputRef) {
        this.props.inputRef.current = input;
    }
}

TODO

Refactor rest of MenuItems
Clean up PR

This PR currently has no changelog labels, so will not be included in changelogs.

A reviewer can add one of: T-Deprecation, T-Enhancement, T-Defect, T-Task to indicate what type of change this is, or add Type: [enhancement/defect/task] to the description and I'll add them for you.

Preview: https://61b30b58b34514d9ad23d6d6--matrix-react-sdk.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.

Part of MSC3030: matrix-org/matrix-spec-proposals#3030 Experimental Synapse implementation added in matrix-org/synapse#9445

MadLittleMods · 2021-12-09T09:41:19Z

res/css/views/context_menus/_IconizedContextMenu.scss

@@ -50,21 +50,21 @@ limitations under the License.
        }

        // round the top corners of the top button for the hover effect to be bounded
-        &:first-child .mx_AccessibleButton:first-child {
+        &:first-child .mx_AccessibleButton:not(.mx_AccessibleButton_hasKind):first-child {


Avoid style clashes when trying to put a primary accessible button inside of a ContextMenu

There is a reason not to do that, it won't be accessible for keyboard-only users. Context Menus have strict focus management requirements

(new better flow suggestions welcome) 🙂

Popping off to a modal feels a little heavy.

Tabbing to a date picker in the context menu seems in the realm of sane to handle for the keyboard. But the HTML5 date picker here isn't giving the best UX because ideally we could remove the "Go" button and "Go" automatically when a date is picked. This doesn't seem feasible though because there no way to distinguish a change from navigating the months of the picker vs a final selection.

Tab in a context menu closes the context menu though as it is focus-locked

I think we can make this work. And seems acceptable under ARIA guidelines:

Tab: Moves focus to the next element in the tab sequence, and if the item that had focus is not in a menubar, closes its menu and all open parent menu containers.

Shift + Tab: Moves focus to the previous element in the tab sequence, and if the item that had focus is not in a menubar, closes its menu and all open parent menu containers.

-- Menu or Menu bar -> Keyboard Interaction, https://www.w3.org/TR/wai-aria-practices/#keyboard-interaction-12

I had this mostly working in 9fa980c except for closing the menu, but Tab was navigating the items as expected.

I think I will try to adapt this to use the RovingTabIndex like the TODO comment in ContextMenu mentions so I can register the items explicitly for navigation and be able to tell when we tab out the end or the start to close the menu.

Is there a RovingTabIndex example that is known working and well implemented that I can base this off of? The spaces panel and room list seems like a pretty similar thing with the tree view being navigable with arrow keys and can Tab to the space options of each as you go.

The MessageActionBar which uses Toolbar and RovingTabIndex seems broken as I can't figure out how to navigate by Tab or Arrow keys in it.

It also seems counter-intuitive to me that I'm unable to navigate menus by Tab and have to use Arrow keys but it also seems inconsistent 🤔. Both seem like they should work.

What's the goal of setting tabindex="-1" on elements which are focusable? Maybe to get around focus holes in the page with lots of elements? It doesn't seem as applicable in a ContextMenu which has to be explicitly opened and will close when tabbed out of though.

It also seems counter-intuitive to me that I'm unable to navigate menus by Tab and have to use Arrow keys but it also seems inconsistent 🤔. Both seem like they should work.

What's the goal of setting tabindex="-1" on elements which are focusable?

See https://developer.mozilla.org/en-US/docs/Web/Accessibility/Keyboard-navigable_JavaScript_widgets Technique 1

It is so the app doesn't have a million tabstops, and instead each widget is a tabstop with sane and expected keyboard & focus management, e.g arrows. The roving & treeview stuff was written to match the wai-aria practices

Switching to roving was asked for by a lot of our a11y community, who have only the keyboard to use the app with and if they have to press tab ten times to get through every menu that just makes any action they want to take that much slower.

MessageActionBar does seem to have regressed.
The space panel, room list, space home are all roving focus managed

#7336 fixes MAB, during a refactoring something got missed I think

Thanks for looking into the fixes for the MessageActionBar! 🌟 I've created a v2 PR, #7339, based off of those fixes and wanting to refactor ContextMenu to use RovingTabIndex so that I can leave this PR untouched in its somewhat working state.

MadLittleMods · 2021-12-09T09:42:03Z

res/css/views/elements/_Field.scss

@@ -127,6 +127,7 @@ limitations under the License.
        transform 0.25s ease-out 0s,
        background-color 0.25s ease-out 0s;
    font-size: $font-10px;
+    line-height: normal;


Reset line-height so it acts normal regardless of environment (ContextMenu has a very large line-height)

MadLittleMods · 2021-12-09T09:42:36Z

src/components/views/messages/DateSeparator.tsx

+            const code = e.errcode || e.statusCode;
+            // only show the dialog if failing for something other than a network error
+            // (e.g. no errcode or statusCode) as in that case the redactions end up in the
+            // detached queue and we show the room status bar to allow retry


TODO: Update comment, copy-paste artifact

This currently works except for closing the menu when you Tab or Shift + Tab out of the boundaries of the menu. > - `Tab`: Moves focus to the next element in the tab sequence, and if the item that had focus is not in a `menubar`, closes its `menu` and all open parent `menu` containers. > > - `Shift + Tab`: Moves focus to the previous element in the tab sequence, and if the item that had focus is not in a `menubar`, closes its `menu` and all open parent `menu` containers. > > https://www.w3.org/TR/wai-aria-practices/#keyboard-interaction-12 I'm going to try to migrate this to using the RovingTabIndex which is mentioned in a TODO comment and will probably make this more sane

HarHarLinks · 2021-12-10T17:36:47Z

You should probably also add this menu in the room info panel or similar, since unlike in your screenshot the closest date header might be pages and pages back if the room is somewhat active.

MadLittleMods · 2021-12-10T18:00:59Z

res/css/views/context_menus/_IconizedContextMenu.scss

@@ -50,21 +50,21 @@ limitations under the License.
        }


You should probably also add this menu in the room info panel or similar, since unlike in your screenshot the closest date header might be pages and pages back if the room is somewhat active.

#7317 (comment)

@HarHarLinks As separate iterations, the plan is to make the date headers sticky so one is always visible.

And add a slash command /jumptodate 2021-12-10

Copy changes from #7317 on top of RovingTabIndex fixes in #7336

Add jump to date functionality to date headers in timeline

594c7c7

Part of MSC3030: matrix-org/matrix-spec-proposals#3030 Experimental Synapse implementation added in matrix-org/synapse#9445

MadLittleMods requested a review from a team as a code owner December 9, 2021 09:40

MadLittleMods marked this pull request as draft December 9, 2021 09:40

MadLittleMods removed the request for review from a team December 9, 2021 09:40

MadLittleMods commented Dec 9, 2021

View reviewed changes

MadLittleMods added 3 commits December 9, 2021 21:42

Navigate after date picker selection (native JS behavior)

23da673

Close the ContextMenu when tabbing away from it

e2d9751

MadLittleMods commented Dec 10, 2021

View reviewed changes

This was referenced Dec 10, 2021

[Design] Hidden sticky day headers in element element-hq/element-web#20142

Closed

Redesign: timeline: make date separator stick to bottom of room header element-hq/element-web#7575

Open

MadLittleMods added a commit that referenced this pull request Dec 11, 2021

Copy Jump to Date PoC changes on top of RovingTabIndex fixes

b061c94

Copy changes from #7317 on top of RovingTabIndex fixes in #7336

MadLittleMods mentioned this pull request Dec 11, 2021

Add jump to date functionality to date headers in timeline v2 #7339

Merged

5 tasks

MadLittleMods closed this Dec 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Draft: Add jump to date functionality to date headers in timeline #7317

Draft: Add jump to date functionality to date headers in timeline #7317

MadLittleMods commented Dec 9, 2021 •

edited by github-actions bot

Loading

MadLittleMods Dec 9, 2021 •

edited

Loading

t3chguy Dec 9, 2021

MadLittleMods Dec 9, 2021

t3chguy Dec 9, 2021

MadLittleMods Dec 10, 2021 •

edited

Loading

MadLittleMods Dec 10, 2021 •

edited

Loading

t3chguy Dec 10, 2021 •

edited

Loading

t3chguy Dec 10, 2021

t3chguy Dec 10, 2021

MadLittleMods Dec 11, 2021 •

edited

Loading

MadLittleMods Dec 9, 2021

MadLittleMods Dec 9, 2021

HarHarLinks commented Dec 10, 2021

MadLittleMods Dec 10, 2021 •

edited

Loading

Draft: Add jump to date functionality to date headers in timeline #7317

Draft: Add jump to date functionality to date headers in timeline #7317

Conversation

MadLittleMods commented Dec 9, 2021 • edited by github-actions bot Loading

Dev notes

TODO

MadLittleMods Dec 9, 2021 • edited Loading

Choose a reason for hiding this comment

t3chguy Dec 9, 2021

Choose a reason for hiding this comment

MadLittleMods Dec 9, 2021

Choose a reason for hiding this comment

t3chguy Dec 9, 2021

Choose a reason for hiding this comment

MadLittleMods Dec 10, 2021 • edited Loading

Choose a reason for hiding this comment

MadLittleMods Dec 10, 2021 • edited Loading

Choose a reason for hiding this comment

t3chguy Dec 10, 2021 • edited Loading

Choose a reason for hiding this comment

t3chguy Dec 10, 2021

Choose a reason for hiding this comment

t3chguy Dec 10, 2021

Choose a reason for hiding this comment

MadLittleMods Dec 11, 2021 • edited Loading

Choose a reason for hiding this comment

MadLittleMods Dec 9, 2021

Choose a reason for hiding this comment

MadLittleMods Dec 9, 2021

Choose a reason for hiding this comment

HarHarLinks commented Dec 10, 2021

MadLittleMods Dec 10, 2021 • edited Loading

Choose a reason for hiding this comment

MadLittleMods commented Dec 9, 2021 •

edited by github-actions bot

Loading

MadLittleMods Dec 9, 2021 •

edited

Loading

MadLittleMods Dec 10, 2021 •

edited

Loading

MadLittleMods Dec 10, 2021 •

edited

Loading

t3chguy Dec 10, 2021 •

edited

Loading

MadLittleMods Dec 11, 2021 •

edited

Loading

MadLittleMods Dec 10, 2021 •

edited

Loading