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

Fix timeline jumping issues related to bubble layout #7529

Merged
merged 6 commits into from
Jan 18, 2022
Merged
Show file tree
Hide file tree
Changes from 5 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
42 changes: 33 additions & 9 deletions res/css/views/rooms/_EventBubbleTile.scss
Original file line number Diff line number Diff line change
Expand Up @@ -215,13 +215,23 @@ limitations under the License.
}

//noinspection CssReplaceWithShorthandSafely
.mx_MImageBody .mx_MImageBody_thumbnail {
// Note: This is intentionally not compressed because the browser gets confused
// when it is all combined. We're effectively unsetting the border radius then
// setting the two corners we care about manually.
border-radius: unset;
border-top-left-radius: var(--cornerRadius);
border-top-right-radius: var(--cornerRadius);
.mx_MImageBody {
width: 100%;
height: 100%;

//noinspection CssReplaceWithShorthandSafely
.mx_MImageBody_thumbnail {
// Note: This is intentionally not compressed because the browser gets confused
// when it is all combined. We're effectively unsetting the border radius then
// setting the two corners we care about manually.
border-radius: unset;
border-top-left-radius: var(--cornerRadius);
border-top-right-radius: var(--cornerRadius);

&.mx_MImageBody_thumbnail--blurhash {
position: unset;
}
}
}

.mx_EventTile_e2eIcon {
Expand Down Expand Up @@ -434,7 +444,6 @@ limitations under the License.
}
&[aria-expanded=true] {
text-align: right;
margin-right: 100px;
}
}

Expand All @@ -457,11 +466,26 @@ limitations under the License.
padding: 0 49px;
}

// ideally we'd use display=contents here for the layout to all work regardless of the *ELS but
// that breaks ScrollPanel's reliance upon offsetTop so we have to have a bit more finesse.
.mx_EventListSummary[data-expanded=true][data-layout=bubble] {
display: contents;
margin: 0;

.mx_EventTile {
padding: 2px 0;
margin-right: 0;

.mx_MessageActionBar {
right: 127px; // align with that of right-column bubbles
}

.mx_EventTile_readAvatars {
right: -18px; // match alignment to RRs of chat bubbles
}

&::before {
right: 0; // match alignment of the hover background to that of chat bubbles
}
}
}

Expand Down
13 changes: 6 additions & 7 deletions src/components/structures/ScrollPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ interface IProps {
* The promise should resolve to true if there is more data to be
* retrieved in this direction (in which case onFillRequest may be
* called again immediately), or false if there is no more data in this
* directon (at this time) - which will stop the pagination cycle until
* direction (at this time) - which will stop the pagination cycle until
* the user scrolls again.
*/
onFillRequest?(backwards: boolean): Promise<boolean>;
Expand Down Expand Up @@ -683,7 +683,7 @@ export default class ScrollPanel extends React.Component<IProps> {
return;
}
const scrollToken = node.dataset.scrollTokens.split(',')[0];
debuglog("saving anchored scroll state to message", node && node.innerText, scrollToken);
debuglog("saving anchored scroll state to message", node.innerText, scrollToken);
const bottomOffset = this.topFromBottom(node);
this.scrollState = {
stuckAtBottom: false,
Expand Down Expand Up @@ -791,17 +791,16 @@ export default class ScrollPanel extends React.Component<IProps> {
const scrollState = this.scrollState;
const trackedNode = scrollState.trackedNode;

if (!trackedNode || !trackedNode.parentElement) {
let node;
if (!trackedNode?.parentElement) {
let node: HTMLElement;
const messages = this.itemlist.current.children;
const scrollToken = scrollState.trackedScrollToken;

for (let i = messages.length-1; i >= 0; --i) {
for (let i = messages.length - 1; i >= 0; --i) {
const m = messages[i] as HTMLElement;
// 'data-scroll-tokens' is a DOMString of comma-separated scroll tokens
// There might only be one scroll token
if (m.dataset.scrollTokens &&
m.dataset.scrollTokens.split(',').indexOf(scrollToken) !== -1) {
if (m.dataset.scrollTokens?.split(',').includes(scrollToken)) {
node = m;
break;
}
Expand Down
22 changes: 11 additions & 11 deletions src/components/structures/TimelinePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ const DEBUG = false;
let debuglog = function(...s: any[]) {};
if (DEBUG) {
// using bind means that we get to keep useful line numbers in the console
debuglog = logger.log.bind(console);
debuglog = logger.log.bind(console, "TimelinePanel debuglog:");
}

interface IProps {
Expand Down Expand Up @@ -239,7 +239,7 @@ class TimelinePanel extends React.Component<IProps, IState> {
constructor(props, context) {
super(props, context);

debuglog("TimelinePanel: mounting");
debuglog("mounting");

// XXX: we could track RM per TimelineSet rather than per Room.
// but for now we just do it per room for simplicity.
Expand Down Expand Up @@ -361,7 +361,7 @@ class TimelinePanel extends React.Component<IProps, IState> {
private onMessageListUnfillRequest = (backwards: boolean, scrollToken: string): void => {
// If backwards, unpaginate from the back (i.e. the start of the timeline)
const dir = backwards ? EventTimeline.BACKWARDS : EventTimeline.FORWARDS;
debuglog("TimelinePanel: unpaginating events in direction", dir);
debuglog("unpaginating events in direction", dir);

// All tiles are inserted by MessagePanel to have a scrollToken === eventId, and
// this particular event should be the first or last to be unpaginated.
Expand All @@ -376,7 +376,7 @@ class TimelinePanel extends React.Component<IProps, IState> {
const count = backwards ? marker + 1 : this.state.events.length - marker;

if (count > 0) {
debuglog("TimelinePanel: Unpaginating", count, "in direction", dir);
debuglog("Unpaginating", count, "in direction", dir);
this.timelineWindow.unpaginate(count, backwards);

const { events, liveEvents, firstVisibleEventIndex } = this.getEvents();
Expand Down Expand Up @@ -417,28 +417,28 @@ class TimelinePanel extends React.Component<IProps, IState> {
const paginatingKey = backwards ? 'backPaginating' : 'forwardPaginating';

if (!this.state[canPaginateKey]) {
debuglog("TimelinePanel: have given up", dir, "paginating this timeline");
debuglog("have given up", dir, "paginating this timeline");
return Promise.resolve(false);
}

if (!this.timelineWindow.canPaginate(dir)) {
debuglog("TimelinePanel: can't", dir, "paginate any further");
debuglog("can't", dir, "paginate any further");
this.setState<null>({ [canPaginateKey]: false });
return Promise.resolve(false);
}

if (backwards && this.state.firstVisibleEventIndex !== 0) {
debuglog("TimelinePanel: won't", dir, "paginate past first visible event");
debuglog("won't", dir, "paginate past first visible event");
return Promise.resolve(false);
}

debuglog("TimelinePanel: Initiating paginate; backwards:"+backwards);
debuglog("Initiating paginate; backwards:"+backwards);
this.setState<null>({ [paginatingKey]: true });

return this.onPaginationRequest(this.timelineWindow, dir, PAGINATE_SIZE).then((r) => {
if (this.unmounted) { return; }

debuglog("TimelinePanel: paginate complete backwards:"+backwards+"; success:"+r);
debuglog("paginate complete backwards:"+backwards+"; success:"+r);

const { events, liveEvents, firstVisibleEventIndex } = this.getEvents();
const newState: Partial<IState> = {
Expand All @@ -455,7 +455,7 @@ class TimelinePanel extends React.Component<IProps, IState> {
const canPaginateOtherWayKey = backwards ? 'canForwardPaginate' : 'canBackPaginate';
if (!this.state[canPaginateOtherWayKey] &&
this.timelineWindow.canPaginate(otherDirection)) {
debuglog('TimelinePanel: can now', otherDirection, 'paginate again');
debuglog('can now', otherDirection, 'paginate again');
newState[canPaginateOtherWayKey] = true;
}

Expand Down Expand Up @@ -781,7 +781,7 @@ class TimelinePanel extends React.Component<IProps, IState> {
const roomId = this.props.timelineSet.room.roomId;
const hiddenRR = SettingsStore.getValue("feature_hidden_read_receipts", roomId);

debuglog('TimelinePanel: Sending Read Markers for ',
debuglog('Sending Read Markers for ',
this.props.timelineSet.room.roomId,
'rm', this.state.readMarkerEventId,
lastReadEvent ? 'rr ' + lastReadEvent.getId() : '',
Expand Down
14 changes: 7 additions & 7 deletions src/components/views/messages/MImageBody.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -338,8 +338,8 @@ export default class MImageBody extends React.Component<IBodyProps, IState> {
content: IMediaEventContent,
forcedHeight?: number,
): JSX.Element {
let infoWidth;
let infoHeight;
let infoWidth: number;
let infoHeight: number;

if (content && content.info && content.info.w && content.info.h) {
infoWidth = content.info.w;
Expand Down Expand Up @@ -382,8 +382,8 @@ export default class MImageBody extends React.Component<IBodyProps, IState> {
const suggestedAndPossibleHeight = Math.min(suggestedImageSize(imageSize, isPortrait).h, infoHeight);
const aspectRatio = infoWidth / infoHeight;

let maxWidth;
let maxHeight;
let maxWidth: number;
let maxHeight: number;
const maxHeightConstraint = forcedHeight || this.props.maxImageHeight || suggestedAndPossibleHeight;
if (maxHeightConstraint * aspectRatio < suggestedAndPossibleWidth || imageSize === ImageSize.Large) {
// The width is dictated by the maximum height that was defined by the props or the function param `forcedHeight`
Expand Down Expand Up @@ -451,7 +451,7 @@ export default class MImageBody extends React.Component<IBodyProps, IState> {
// This has incredibly broken types.
const C = CSSTransition as any;
const thumbnail = (
<div className="mx_MImageBody_thumbnail_container" style={{ maxHeight: maxHeight, maxWidth: maxWidth, aspectRatio: `${infoWidth}/${infoHeight}` }}>
<div className="mx_MImageBody_thumbnail_container" style={{ maxHeight, maxWidth, aspectRatio: `${infoWidth}/${infoHeight}` }}>
<SwitchTransition mode="out-in">
<C
classNames="mx_rtg--fade"
Expand All @@ -464,8 +464,8 @@ export default class MImageBody extends React.Component<IBodyProps, IState> {
className={classes}
style={{
// Constrain width here so that spinner appears central to the loaded thumbnail
maxWidth: `min(100%, ${infoWidth}px)`,
maxHeight: maxHeight,
maxWidth,
maxHeight,
aspectRatio: `${infoWidth}/${infoHeight}`,
}}
>
Expand Down