Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

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

Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions res/css/views/context_menus/_IconizedContextMenu.scss
Original file line number Diff line number Diff line change
Expand Up @@ -50,21 +50,21 @@ limitations under the License.
}
Copy link
Contributor Author

@MadLittleMods MadLittleMods Dec 10, 2021

Choose a reason for hiding this comment

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

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


// 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 {
Copy link
Contributor Author

@MadLittleMods MadLittleMods Dec 9, 2021

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(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.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@MadLittleMods MadLittleMods Dec 10, 2021

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@MadLittleMods MadLittleMods Dec 10, 2021

Choose a reason for hiding this comment

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

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.

Copy link
Member

@t3chguy t3chguy Dec 10, 2021

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@MadLittleMods MadLittleMods Dec 11, 2021

Choose a reason for hiding this comment

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

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.

border-radius: 8px 8px 0 0; // radius matches .mx_ContextualMenu
}

// round the bottom corners of the bottom button for the hover effect to be bounded
&:last-child .mx_AccessibleButton:last-child {
&:last-child .mx_AccessibleButton:not(.mx_AccessibleButton_hasKind):last-child {
border-radius: 0 0 8px 8px; // radius matches .mx_ContextualMenu
}

// round all corners of the only button for the hover effect to be bounded
&:first-child:last-child .mx_AccessibleButton:first-child:last-child {
&:first-child:last-child .mx_AccessibleButton:not(.mx_AccessibleButton_hasKind):first-child:last-child {
border-radius: 8px; // radius matches .mx_ContextualMenu
}

.mx_AccessibleButton {
.mx_AccessibleButton:not(.mx_AccessibleButton_hasKind) {
// pad the inside of the button so that the hover background is padded too
padding-top: 12px;
padding-bottom: 12px;
Expand Down Expand Up @@ -130,7 +130,7 @@ limitations under the License.
}

.mx_IconizedContextMenu_optionList_red {
.mx_AccessibleButton {
.mx_AccessibleButton:not(.mx_AccessibleButton_hasKind) {
color: $alert !important;
}

Expand All @@ -148,7 +148,7 @@ limitations under the License.
}

.mx_IconizedContextMenu_active {
&.mx_AccessibleButton, .mx_AccessibleButton {
&.mx_AccessibleButton:not(.mx_AccessibleButton_hasKind), .mx_AccessibleButton:not(.mx_AccessibleButton_hasKind) {
color: $accent !important;
}

Expand Down
1 change: 1 addition & 0 deletions res/css/views/elements/_Field.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

transform: translateY(-13px);
padding: 0 2px;
background-color: $background;
Expand Down
34 changes: 34 additions & 0 deletions res/css/views/messages/_DateSeparator.scss
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,37 @@ limitations under the License.
margin: 0 25px;
flex: 0 0 auto;
}

.mx_DateSeparator_jumpToDateMenu {
display: flex;
}

.mx_DateSeparator_chevron {
align-self: center;
width: 16px;
height: 16px;
mask-position: center;
mask-size: contain;
mask-repeat: no-repeat;
mask-image: url('$(res)/img/feather-customised/chevron-down.svg');
background-color: $tertiary-content;
}

.mx_DateSeparator_jumpToDateMenuOption > .mx_IconizedContextMenu_label {
flex: initial;
width: auto;
}

.mx_DateSeparator_datePickerForm {
display: flex;
}

.mx_DateSeparator_datePicker {
flex: initial;
margin: 0;
margin-left: 8px;
}

.mx_DateSeparator_datePickerSubmitButton {
margin-left: 8px;
}
10 changes: 5 additions & 5 deletions src/components/structures/MessagePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -715,8 +715,8 @@ export default class MessagePanel extends React.Component<IProps, IState> {

// do we need a date separator since the last event?
const wantsDateSeparator = this.wantsDateSeparator(prevEvent, eventDate);
if (wantsDateSeparator && !isGrouped) {
const dateSeparator = <li key={ts1}><DateSeparator key={ts1} ts={ts1} /></li>;
if (wantsDateSeparator && !isGrouped && this.props.room) {
const dateSeparator = <li key={ts1}><DateSeparator key={ts1} roomId={this.props.room.roomId} ts={ts1} /></li>;
ret.push(dateSeparator);
}

Expand Down Expand Up @@ -1109,7 +1109,7 @@ class CreationGrouper extends BaseGrouper {
if (panel.wantsDateSeparator(this.prevEvent, createEvent.getDate())) {
const ts = createEvent.getTs();
ret.push(
<li key={ts+'~'}><DateSeparator key={ts+'~'} ts={ts} /></li>,
<li key={ts+'~'}><DateSeparator key={ts+'~'} roomId={createEvent.getRoomId()} ts={ts} /></li>,
);
}

Expand Down Expand Up @@ -1222,7 +1222,7 @@ class RedactionGrouper extends BaseGrouper {
if (panel.wantsDateSeparator(this.prevEvent, this.events[0].getDate())) {
const ts = this.events[0].getTs();
ret.push(
<li key={ts+'~'}><DateSeparator key={ts+'~'} ts={ts} /></li>,
<li key={ts+'~'}><DateSeparator key={ts+'~'} roomId={this.events[0].getRoomId()} ts={ts} /></li>,
);
}

Expand Down Expand Up @@ -1318,7 +1318,7 @@ class MemberGrouper extends BaseGrouper {
if (panel.wantsDateSeparator(this.prevEvent, this.events[0].getDate())) {
const ts = this.events[0].getTs();
ret.push(
<li key={ts+'~'}><DateSeparator key={ts+'~'} ts={ts} /></li>,
<li key={ts+'~'}><DateSeparator key={ts+'~'} roomId={this.events[0].getRoomId()} ts={ts} /></li>,
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ export default class MessageEditHistoryDialog extends React.PureComponent<IProps
const baseEventId = this.props.mxEvent.getId();
allEvents.forEach((e, i) => {
if (!lastEvent || wantsDateSeparator(lastEvent.getDate(), e.getDate())) {
nodes.push(<li key={e.getTs() + "~"}><DateSeparator ts={e.getTs()} /></li>);
nodes.push(<li key={e.getTs() + "~"}><DateSeparator roomId={e.getRoomId()} ts={e.getTs()} /></li>);
}
const isBaseEvent = e.getId() === baseEventId;
nodes.push((
Expand Down
193 changes: 189 additions & 4 deletions src/components/views/messages/DateSeparator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,22 @@ import React from 'react';
import { _t } from '../../../languageHandler';
import { formatFullDateNoTime } from '../../../DateUtils';
import { replaceableComponent } from "../../../utils/replaceableComponent";
import { MatrixClientPeg } from '../../../MatrixClientPeg';
import { Direction } from 'matrix-js-sdk/src/models/event-timeline';
import dis from '../../../dispatcher/dispatcher';
import { Action } from '../../../dispatcher/actions';

import Field from "../elements/Field";
import Modal from '../../../Modal';
import ErrorDialog from '../dialogs/ErrorDialog';
import AccessibleButton from "../elements/AccessibleButton";
import { contextMenuBelow } from '../rooms/RoomTile';
import { ContextMenuTooltipButton } from "../../structures/ContextMenu";
import IconizedContextMenu, {
IconizedContextMenuOption,
IconizedContextMenuOptionList,
IconizedContextMenuRadio,
} from "../context_menus/IconizedContextMenu";

function getDaysArray(): string[] {
return [
Expand All @@ -34,13 +50,26 @@ function getDaysArray(): string[] {
}

interface IProps {
roomId: string,
ts: number;
forExport?: boolean;
}

interface IState {
dateValue: string,
contextMenuPosition?: DOMRect
}

@replaceableComponent("views.messages.DateSeparator")
export default class DateSeparator extends React.Component<IProps> {
private getLabel() {
export default class DateSeparator extends React.Component<IProps, IState> {
constructor(props, context) {
super(props, context);
this.state = {
dateValue: this.getDefaultDateValue()
};
}

private getLabel(): string {
const date = new Date(this.props.ts);

// During the time the archive is being viewed, a specific day might not make sense, so we return the full date
Expand All @@ -62,12 +91,168 @@ export default class DateSeparator extends React.Component<IProps> {
}
}

private getDefaultDateValue(): string {
const date = new Date(this.props.ts);
const year = date.getFullYear();
const month = `${date.getMonth() + 1}`.padStart(2, "0")
const day = `${date.getDate()}`.padStart(2, "0")

return `${year}-${month}-${day}`
}

private pickDate = async (inputTimestamp): Promise<void> => {
console.log('pickDate', inputTimestamp)

const unixTimestamp = new Date(inputTimestamp).getTime();

const cli = MatrixClientPeg.get();
try {
const roomId = this.props.roomId
const { event_id, origin_server_ts } = await cli.timestampToEvent(
roomId,
unixTimestamp,
Direction.Forward
);
console.log(`/timestamp_to_event: found ${event_id} (${origin_server_ts}) for timestamp=${unixTimestamp}`)

dis.dispatch({
action: Action.ViewRoom,
event_id,
highlighted: true,
room_id: roomId,
});
} catch (e) {
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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: Update comment, copy-paste artifact

if (typeof code !== "undefined") {
// display error message stating you couldn't delete this.
Modal.createTrackedDialog('Unable to find event at that date', '', ErrorDialog, {
title: _t('Error'),
description: _t('Unable to find event at that date. (%(code)s)', { code }),
});
}
}
};

private onDateValueChange = (e: React.ChangeEvent<HTMLSelectElement | HTMLInputElement>): void => {
this.setState({ dateValue: e.target.value });
};

private onContextMenuOpenClick = (ev: React.MouseEvent): void => {
ev.preventDefault();
ev.stopPropagation();
const target = ev.target as HTMLButtonElement;
this.setState({ contextMenuPosition: target.getBoundingClientRect() });
};

private closeMenu = (): void => {
this.setState({
contextMenuPosition: null,
});
};

private onContextMenuCloseClick = (): void => {
this.closeMenu();
};

private onLastWeekClicked = (): void => {
const date = new Date();
// This just goes back 7 days.
// FIXME: Do we want this to go back to the last Sunday? https://upokary.com/how-to-get-last-monday-or-last-friday-or-any-last-day-in-javascript/
date.setDate(date.getDate() - 7);
this.pickDate(date);
this.closeMenu();
}

private onLastMonthClicked = (): void => {
const date = new Date();
// Month numbers are 0 - 11 and `setMonth` handles the negative rollover
date.setMonth(date.getMonth() - 1, 1);
this.pickDate(date);
this.closeMenu();
}

private onTheBeginningClicked = (): void => {
const date = new Date(0);
this.pickDate(date);
this.closeMenu();
}

private onJumpToDateSubmit = (): void => {
console.log('onJumpToDateSubmit')
this.pickDate(this.state.dateValue);
this.closeMenu();
}

private renderNotificationsMenu(): React.ReactElement {
let contextMenu: JSX.Element;
if (this.state.contextMenuPosition) {
contextMenu = <IconizedContextMenu
{...contextMenuBelow(this.state.contextMenuPosition)}
compact
onFinished={this.onContextMenuCloseClick}
>
<IconizedContextMenuOptionList first>
<IconizedContextMenuOption
label={_t("Last week")}
onClick={this.onLastWeekClicked}
/>
<IconizedContextMenuOption
label={_t("Last month")}
onClick={this.onLastMonthClicked}
/>
<IconizedContextMenuOption
label={_t("The beginning of the room")}
onClick={this.onTheBeginningClicked}
/>
</IconizedContextMenuOptionList>

<IconizedContextMenuOptionList>
<IconizedContextMenuOption
className="mx_DateSeparator_jumpToDateMenuOption"
label={_t("Jump to date")}
onClick={() => {}}
>
<form className="mx_DateSeparator_datePickerForm" onSubmit={this.onJumpToDateSubmit}>
<Field
type="date"
onChange={this.onDateValueChange}
value={this.state.dateValue}
className="mx_DateSeparator_datePicker"
label={_t("Pick a date to jump to")}
autoFocus={true}
/>
<AccessibleButton kind="primary" className="mx_DateSeparator_datePickerSubmitButton" onClick={this.onJumpToDateSubmit}>
{ _t("Go") }
</AccessibleButton>
</form>
</IconizedContextMenuOption>
</IconizedContextMenuOptionList>
</IconizedContextMenu>;
}

return (
<ContextMenuTooltipButton
className="mx_DateSeparator_jumpToDateMenu"
onClick={this.onContextMenuOpenClick}
isExpanded={!!this.state.contextMenuPosition}
title={_t("Jump to date")}
>
<div aria-hidden="true">{ this.getLabel() }</div>
<div className="mx_DateSeparator_chevron" />
{ contextMenu }
</ContextMenuTooltipButton>
);
}

render() {
// ARIA treats <hr/>s as separators, here we abuse them slightly so manually treat this entire thing as one
// tab-index=-1 to allow it to be focusable but do not add tab stop for it, primarily for screen readers
return <h2 className="mx_DateSeparator" role="separator" tabIndex={-1} aria-label={this.getLabel()}>
return <h2 className="mx_DateSeparator" role="separator" aria-label={this.getLabel()}>
<hr role="none" />
<div aria-hidden="true">{ this.getLabel() }</div>
{ this.renderNotificationsMenu() }
<hr role="none" />
</h2>;
}
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/rooms/SearchResultTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export default class SearchResultTile extends React.Component<IProps> {
const eventId = mxEv.getId();

const ts1 = mxEv.getTs();
const ret = [<DateSeparator key={ts1 + "-search"} ts={ts1} />];
const ret = [<DateSeparator key={ts1 + "-search"} roomId={mxEv.getRoomId()} ts={ts1} />];
const layout = SettingsStore.getValue("layout");
const isTwelveHour = SettingsStore.getValue("showTwelveHourTimestamps");
const alwaysShowTimestamps = SettingsStore.getValue("alwaysShowTimestamps");
Expand Down
8 changes: 7 additions & 1 deletion src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -2026,6 +2026,13 @@
"Saturday": "Saturday",
"Today": "Today",
"Yesterday": "Yesterday",
"Unable to find event at that date. (%(code)s)": "Unable to find event at that date. (%(code)s)",
"Last week": "Last week",
"Last month": "Last month",
"The beginning of the room": "The beginning of the room",
"Jump to date": "Jump to date",
"Pick a date to jump to": "Pick a date to jump to",
"Go": "Go",
"Downloading": "Downloading",
"Decrypting": "Decrypting",
"Download": "Download",
Expand Down Expand Up @@ -2550,7 +2557,6 @@
"Start a conversation with someone using their name, email address or username (like <userId/>).": "Start a conversation with someone using their name, email address or username (like <userId/>).",
"Start a conversation with someone using their name or username (like <userId/>).": "Start a conversation with someone using their name or username (like <userId/>).",
"This won't invite them to %(communityName)s. To invite someone to %(communityName)s, click <a>here</a>": "This won't invite them to %(communityName)s. To invite someone to %(communityName)s, click <a>here</a>",
"Go": "Go",
"Some suggestions may be hidden for privacy.": "Some suggestions may be hidden for privacy.",
"If you can't see who you're looking for, send them your invite link below.": "If you can't see who you're looking for, send them your invite link below.",
"Or send invite link": "Or send invite link",
Expand Down
2 changes: 1 addition & 1 deletion src/utils/exportUtils/HtmlExport.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ export default class HTMLExporter extends Exporter {

protected getDateSeparator(event: MatrixEvent) {
const ts = event.getTs();
const dateSeparator = <li key={ts}><DateSeparator forExport={true} key={ts} ts={ts} /></li>;
const dateSeparator = <li key={ts}><DateSeparator forExport={true} key={ts} roomId={event.getRoomId()} ts={ts} /></li>;
return renderToStaticMarkup(dateSeparator);
}

Expand Down