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

Render timeline separator for late event groups #11739

Merged
merged 20 commits into from
Oct 16, 2023
Merged
Show file tree
Hide file tree
Changes from 18 commits
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
4 changes: 2 additions & 2 deletions cypress/e2e/editing/editing.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ describe("Editing", () => {
cy.get(".mx_EventTile").should("have.css", "padding-block-start", "0px");

// Assert that the date separator is rendered at the top
cy.get("li:nth-child(1) .mx_DateSeparator").within(() => {
cy.get("li:nth-child(1) .mx_TimelineSeparator").within(() => {
cy.get("h2").within(() => {
cy.findByText("today").should("have.css", "text-transform", "capitalize");
});
Expand Down Expand Up @@ -182,7 +182,7 @@ describe("Editing", () => {
// Assert that the message edit history dialog is rendered again
cy.get(".mx_MessageEditHistoryDialog").within(() => {
// Assert that the date is rendered
cy.get("li:nth-child(1) .mx_DateSeparator").within(() => {
cy.get("li:nth-child(1) .mx_TimelineSeparator").within(() => {
cy.get("h2").within(() => {
cy.findByText("today").should("have.css", "text-transform", "capitalize");
});
Expand Down
1 change: 1 addition & 0 deletions res/css/_components.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@
@import "./views/messages/_RedactedBody.pcss";
@import "./views/messages/_RoomAvatarEvent.pcss";
@import "./views/messages/_TextualEvent.pcss";
@import "./views/messages/_TimelineSeparator.pcss";
@import "./views/messages/_UnknownBody.pcss";
@import "./views/messages/_ViewSourceEvent.pcss";
@import "./views/messages/_common_CryptoEvent.pcss";
Expand Down
16 changes: 0 additions & 16 deletions res/css/views/messages/_DateSeparator.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,6 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

.mx_DateSeparator {
clear: both;
margin: 4px 0;
display: flex;
align-items: center;
font: var(--cpd-font-body-md-regular);
color: $roomtopic-color;
}

.mx_DateSeparator > hr {
flex: 1 1 0;
height: 0;
border: none;
border-bottom: 1px solid $menu-selected-color;
}

.mx_DateSeparator_dateContent {
padding: 0 25px;
}
Expand Down
31 changes: 31 additions & 0 deletions res/css/views/messages/_TimelineSeparator.pcss
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
Copyright 2017 Vector Creations Ltd
t3chguy marked this conversation as resolved.
Show resolved Hide resolved

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

.mx_TimelineSeparator {
clear: both;
margin: 4px 0;
display: flex;
align-items: center;
font: var(--cpd-font-body-md-regular);
color: $roomtopic-color;
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like a misuse of $roomtopic-color. Should probably be text secondary or something along those lines

Copy link
Member Author

@t3chguy t3chguy Oct 16, 2023

Choose a reason for hiding this comment

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

These styles are not new, they're just broken out of 1 file into 2 files

}

.mx_TimelineSeparator > hr {
flex: 1 1 0;
height: 0;
border: none;
border-bottom: 1px solid $menu-selected-color;
}
2 changes: 1 addition & 1 deletion res/css/views/right_panel/_ThreadPanel.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ limitations under the License.
/* Account for scrollbar when hovering */
padding-top: 0;

.mx_DateSeparator {
.mx_TimelineSeparator {
display: none;
}

Expand Down
80 changes: 52 additions & 28 deletions src/components/structures/MessagePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,9 @@ import classNames from "classnames";
import { Room, MatrixClient, RoomStateEvent, EventStatus, MatrixEvent, EventType } from "matrix-js-sdk/src/matrix";
import { logger } from "matrix-js-sdk/src/logger";
import { isSupportedReceiptType } from "matrix-js-sdk/src/utils";
import { Optional } from "matrix-events-sdk";

import shouldHideEvent from "../../shouldHideEvent";
import { wantsDateSeparator } from "../../DateUtils";
import { formatDate, wantsDateSeparator } from "../../DateUtils";
import { MatrixClientPeg } from "../../MatrixClientPeg";
import SettingsStore from "../../settings/SettingsStore";
import RoomContext, { TimelineRenderingType } from "../../contexts/RoomContext";
Expand All @@ -40,6 +39,7 @@ import LegacyCallEventGrouper from "./LegacyCallEventGrouper";
import WhoIsTypingTile from "../views/rooms/WhoIsTypingTile";
import ScrollPanel, { IScrollState } from "./ScrollPanel";
import DateSeparator from "../views/messages/DateSeparator";
import TimelineSeparator, { SeparatorKind } from "../views/messages/TimelineSeparator";
import ErrorBoundary from "../views/elements/ErrorBoundary";
import ResizeNotifier from "../../utils/ResizeNotifier";
import Spinner from "../views/elements/Spinner";
Expand All @@ -54,6 +54,8 @@ import { hasThreadSummary } from "../../utils/EventUtils";
import { BaseGrouper } from "./grouper/BaseGrouper";
import { MainGrouper } from "./grouper/MainGrouper";
import { CreationGrouper } from "./grouper/CreationGrouper";
import { _t } from "../../languageHandler";
import { getLateEventInfo } from "./grouper/LateEventGrouper";

const CONTINUATION_MAX_INTERVAL = 5 * 60 * 1000; // 5 minutes
const continuedTypes = [EventType.Sticker, EventType.RoomMessage];
Expand Down Expand Up @@ -739,39 +741,46 @@ export default class MessagePanel extends React.Component<IProps, IState> {

const isEditing = this.props.editState?.getEvent().getId() === mxEv.getId();
// local echoes have a fake date, which could even be yesterday. Treat them as 'today' for the date separators.
let ts1 = mxEv.getTs();
let eventDate = mxEv.getDate();
if (mxEv.status) {
eventDate = new Date();
ts1 = eventDate.getTime();
}

// do we need a date separator since the last event?
const wantsDateSeparator = this.wantsDateSeparator(prevEvent, eventDate);
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);
const ts1 = mxEv.getTs() ?? Date.now();

// do we need a separator since the last event?
const wantsSeparator = this.wantsSeparator(prevEvent, mxEv);
if (!isGrouped && this.props.room) {
if (wantsSeparator === SeparatorKind.Date) {
ret.push(
<li key={ts1}>
<DateSeparator key={ts1} roomId={this.props.room.roomId} ts={ts1} />
</li>,
);
} else if (wantsSeparator === SeparatorKind.LateEvent) {
const text = _t("timeline|late_event_separator", {
dateTime: formatDate(mxEv.getDate() ?? new Date()),
});
ret.push(
<li key={ts1}>
<TimelineSeparator key={ts1} label={text}>
{text}
</TimelineSeparator>
</li>,
);
}
}

const cli = MatrixClientPeg.safeGet();
let lastInSection = true;
if (nextEventWithTile) {
const nextEv = nextEventWithTile;
const willWantDateSeparator = this.wantsDateSeparator(mxEv, nextEv.getDate() || new Date());
const willWantSeparator = this.wantsSeparator(mxEv, nextEv);
lastInSection =
willWantDateSeparator ||
willWantSeparator === SeparatorKind.Date ||
mxEv.getSender() !== nextEv.getSender() ||
getEventDisplayInfo(cli, nextEv, this.showHiddenEvents).isInfoMessage ||
!shouldFormContinuation(mxEv, nextEv, cli, this.showHiddenEvents, this.context.timelineRenderingType);
}

// is this a continuation of the previous message?
const continuation =
!wantsDateSeparator &&
wantsSeparator === SeparatorKind.None &&
shouldFormContinuation(prevEvent, mxEv, cli, this.showHiddenEvents, this.context.timelineRenderingType);

const eventId = mxEv.getId()!;
Expand Down Expand Up @@ -816,16 +825,31 @@ export default class MessagePanel extends React.Component<IProps, IState> {
return ret;
}

public wantsDateSeparator(prevEvent: MatrixEvent | null, nextEventDate: Optional<Date>): boolean {
public wantsSeparator(prevEvent: MatrixEvent | null, mxEvent: MatrixEvent): SeparatorKind {
if (this.context.timelineRenderingType === TimelineRenderingType.ThreadsList) {
return false;
return SeparatorKind.None;
}
if (prevEvent == null) {
// first event in the panel: depends if we could back-paginate from
// here.
return !this.props.canBackPaginate;

if (prevEvent !== null) {
// If the previous event was late but current is not then show a date separator for orientation
// Otherwise if the current event is of a different late group than the previous show a late event separator
const lateEventInfo = getLateEventInfo(mxEvent);
if (lateEventInfo?.group_id !== getLateEventInfo(prevEvent)?.group_id) {
return lateEventInfo !== undefined ? SeparatorKind.LateEvent : SeparatorKind.Date;
}
}

// first event in the panel: depends on if we could back-paginate from here.
if (prevEvent === null && !this.props.canBackPaginate) {
return SeparatorKind.Date;
}
return wantsDateSeparator(prevEvent.getDate() || undefined, nextEventDate);

const nextEventDate = mxEvent.getDate() ?? new Date();
if (prevEvent !== null && wantsDateSeparator(prevEvent.getDate() || undefined, nextEventDate)) {
return SeparatorKind.Date;
}

return SeparatorKind.None;
}

// Get a list of read receipts that should be shown next to this event
Expand Down
5 changes: 3 additions & 2 deletions src/components/structures/grouper/CreationGrouper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { _t } from "../../../languageHandler";
import DateSeparator from "../../views/messages/DateSeparator";
import NewRoomIntro from "../../views/rooms/NewRoomIntro";
import GenericEventListSummary from "../../views/elements/GenericEventListSummary";
import { SeparatorKind } from "../../views/messages/TimelineSeparator";

// Wrap initial room creation events into a GenericEventListSummary
// Grouping only events sent by the same user that sent the `m.room.create` and only until
Expand All @@ -41,7 +42,7 @@ export class CreationGrouper extends BaseGrouper {
if (!shouldShow) {
return true;
}
if (panel.wantsDateSeparator(this.firstEventAndShouldShow.event, event.getDate())) {
if (panel.wantsSeparator(this.firstEventAndShouldShow.event, event) === SeparatorKind.Date) {
return false;
}
const eventType = event.getType();
Expand Down Expand Up @@ -96,7 +97,7 @@ export class CreationGrouper extends BaseGrouper {
const createEvent = this.firstEventAndShouldShow;
const lastShownEvent = this.lastShownEvent;

if (panel.wantsDateSeparator(this.prevEvent, createEvent.event.getDate())) {
if (panel.wantsSeparator(this.prevEvent, createEvent.event) === SeparatorKind.Date) {
const ts = createEvent.event.getTs();
ret.push(
<li key={ts + "~"}>
Expand Down
28 changes: 28 additions & 0 deletions src/components/structures/grouper/LateEventGrouper.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
Copyright 2023 The Matrix.org Foundation C.I.C.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

import { MatrixEvent } from "matrix-js-sdk/src/matrix";

const UNSIGNED_KEY = "io.element.late_event";

interface UnsignedLateEventInfo {
received_at: number;
group_id: string;
}

export function getLateEventInfo(mxEvent: MatrixEvent): UnsignedLateEventInfo | undefined {
t3chguy marked this conversation as resolved.
Show resolved Hide resolved
return mxEvent.getUnsigned()[UNSIGNED_KEY];
}
5 changes: 3 additions & 2 deletions src/components/structures/grouper/MainGrouper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { MatrixClientPeg } from "../../../MatrixClientPeg";
import DateSeparator from "../../views/messages/DateSeparator";
import HistoryTile from "../../views/rooms/HistoryTile";
import EventListSummary from "../../views/elements/EventListSummary";
import { SeparatorKind } from "../../views/messages/TimelineSeparator";

const groupedStateEvents = [
EventType.RoomMember,
Expand Down Expand Up @@ -70,7 +71,7 @@ export class MainGrouper extends BaseGrouper {
// absorb hidden events so that they do not break up streams of messages & redaction events being grouped
return true;
}
if (this.panel.wantsDateSeparator(this.events[0].event, ev.getDate())) {
if (this.panel.wantsSeparator(this.events[0].event, ev) === SeparatorKind.Date) {
return false;
}
if (ev.isState() && groupedStateEvents.includes(ev.getType() as EventType)) {
Expand Down Expand Up @@ -114,7 +115,7 @@ export class MainGrouper extends BaseGrouper {
const lastShownEvent = this.lastShownEvent;
const ret: ReactNode[] = [];

if (panel.wantsDateSeparator(this.prevEvent, this.events[0].event.getDate())) {
if (panel.wantsSeparator(this.prevEvent, this.events[0].event) === SeparatorKind.Date) {
const ts = this.events[0].event.getTs();
ret.push(
<li key={ts + "~"}>
Expand Down
10 changes: 2 additions & 8 deletions src/components/views/messages/DateSeparator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import IconizedContextMenu, {
import JumpToDatePicker from "./JumpToDatePicker";
import { ViewRoomPayload } from "../../../dispatcher/payloads/ViewRoomPayload";
import { SdkContextClass } from "../../../contexts/SDKContext";
import TimelineSeparator from "./TimelineSeparator";

interface IProps {
roomId: string;
Expand Down Expand Up @@ -328,13 +329,6 @@ export default class DateSeparator extends React.Component<IProps, IState> {
);
}

// ARIA treats <hr/>s as separators, here we abuse them slightly so manually treat this entire thing as one
return (
<div className="mx_DateSeparator" role="separator" aria-label={label}>
<hr role="none" />
{dateHeaderContent}
<hr role="none" />
</div>
);
return <TimelineSeparator label={label}>{dateHeaderContent}</TimelineSeparator>;
}
}
40 changes: 40 additions & 0 deletions src/components/views/messages/TimelineSeparator.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
Copyright 2023 The Matrix.org Foundation C.I.C.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

import React from "react";

interface Props {
label: string;
}

export const enum SeparatorKind {
None,
Date,
LateEvent,
}

const TimelineSeparator: React.FC<Props> = ({ label, children }) => {
t3chguy marked this conversation as resolved.
Show resolved Hide resolved
// ARIA treats <hr/>s as separators, here we abuse them slightly so manually treat this entire thing as one
Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds like we could and probably should use pseudo elements ::before and ::after in place of <hr /> here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, possibly, this isn't new code, just moving home

return (
<div className="mx_TimelineSeparator" role="separator" aria-label={label}>
<hr role="none" />
{children}
<hr role="none" />
</div>
);
};

export default TimelineSeparator;
3 changes: 2 additions & 1 deletion src/components/views/rooms/EventTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ import { isLocalRoom } from "../../../utils/localRoom/isLocalRoom";
import { ElementCall } from "../../../models/Call";
import { UnreadNotificationBadge } from "./NotificationBadge/UnreadNotificationBadge";
import { EventTileThreadToolbar } from "./EventTile/EventTileThreadToolbar";
import { getLateEventInfo } from "../../structures/grouper/LateEventGrouper";

export type GetRelationsForEvent = (
eventId: string,
Expand Down Expand Up @@ -1126,7 +1127,7 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
showRelative={this.context.timelineRenderingType === TimelineRenderingType.ThreadsList}
showTwelveHour={this.props.isTwelveHour}
ts={ts}
receivedTs={this.props.mxEvent.getUnsigned()["io.element.late_event"]?.received_at}
receivedTs={getLateEventInfo(this.props.mxEvent)?.received_at}
/>
);

Expand Down
1 change: 1 addition & 0 deletions src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -3203,6 +3203,7 @@
"you": "You ended a <a>voice broadcast</a>"
},
"io.element.widgets.layout": "%(senderName)s has updated the room layout",
"late_event_separator": "Originally sent %(dateTime)s",
"load_error": {
"no_permission": "Tried to load a specific point in this room's timeline, but you do not have permission to view the message in question.",
"title": "Failed to load timeline position",
Expand Down
Loading