Fix for message action bar visibility getting out of sync with the real UI state (#33445)
* Make sure action bar is not visible when using up/down arrows during editing * Add a temporary mouse move listener to handle tooltips stealing onMouseLeave events * Better naming, add comments and test * Fix test that performs its own hover/pointer movement before clicking. * Fix playwright test that actually displayed a message time stamp when hover state was stale * Fixes after merge
This commit is contained in:
BIN
Binary file not shown.
|
Before Width: | Height: | Size: 6.2 KiB After Width: | Height: | Size: 6.2 KiB |
@@ -338,6 +338,7 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
|
|||||||
|
|
||||||
private unmounted = false;
|
private unmounted = false;
|
||||||
private readonly id = uniqueId();
|
private readonly id = uniqueId();
|
||||||
|
private staleHoverCheckActive = false;
|
||||||
|
|
||||||
public constructor(props: EventTileProps, context: React.ContextType<typeof RoomContext>) {
|
public constructor(props: EventTileProps, context: React.ContextType<typeof RoomContext>) {
|
||||||
super(props, context);
|
super(props, context);
|
||||||
@@ -472,6 +473,7 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
|
|||||||
}
|
}
|
||||||
|
|
||||||
public componentWillUnmount(): void {
|
public componentWillUnmount(): void {
|
||||||
|
this.stopStaleHoverCheck();
|
||||||
const client = MatrixClientPeg.get();
|
const client = MatrixClientPeg.get();
|
||||||
if (client) {
|
if (client) {
|
||||||
client.removeListener(CryptoEvent.UserTrustStatusChanged, this.onUserVerificationChanged);
|
client.removeListener(CryptoEvent.UserTrustStatusChanged, this.onUserVerificationChanged);
|
||||||
@@ -491,6 +493,14 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
|
|||||||
}
|
}
|
||||||
|
|
||||||
public componentDidUpdate(prevProps: Readonly<EventTileProps>, prevState: Readonly<IState>): void {
|
public componentDidUpdate(prevProps: Readonly<EventTileProps>, prevState: Readonly<IState>): void {
|
||||||
|
// Some overlays, such as portalled tooltips, can interrupt the normal mouseleave path.
|
||||||
|
// While hover is active, verify it against the browser's real :hover state on mouse movement.
|
||||||
|
if (!prevState.hover && this.state.hover) {
|
||||||
|
this.startStaleHoverCheck();
|
||||||
|
} else if (prevState.hover && !this.state.hover) {
|
||||||
|
this.stopStaleHoverCheck();
|
||||||
|
}
|
||||||
|
|
||||||
// If we're not listening for receipts and expect to be, register a listener.
|
// If we're not listening for receipts and expect to be, register a listener.
|
||||||
if (!this.isListeningForReceipts && (this.shouldShowSentReceipt || this.shouldShowSendingReceipt)) {
|
if (!this.isListeningForReceipts && (this.shouldShowSentReceipt || this.shouldShowSendingReceipt)) {
|
||||||
MatrixClientPeg.safeGet().on(RoomEvent.Receipt, this.onRoomReceipt);
|
MatrixClientPeg.safeGet().on(RoomEvent.Receipt, this.onRoomReceipt);
|
||||||
@@ -502,6 +512,17 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (this.props.resizeObserver && this.ref.current) this.props.resizeObserver.observe(this.ref.current);
|
if (this.props.resizeObserver && this.ref.current) this.props.resizeObserver.observe(this.ref.current);
|
||||||
|
|
||||||
|
// Moving between edited messages can remount the editor without a reliable blur event.
|
||||||
|
// Clear stale focus-derived action bar state when focus has actually left this tile.
|
||||||
|
if (
|
||||||
|
this.state.focusWithin &&
|
||||||
|
this.ref.current &&
|
||||||
|
document.activeElement instanceof HTMLElement &&
|
||||||
|
!this.ref.current.contains(document.activeElement)
|
||||||
|
) {
|
||||||
|
this.setState({ focusWithin: false, showActionBarFromFocus: false });
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private readonly onNewThread = (thread: Thread): void => {
|
private readonly onNewThread = (thread: Thread): void => {
|
||||||
@@ -868,6 +889,32 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
|
|||||||
}));
|
}));
|
||||||
};
|
};
|
||||||
|
|
||||||
|
private startStaleHoverCheck(): void {
|
||||||
|
if (this.staleHoverCheckActive) return;
|
||||||
|
document.addEventListener("mousemove", this.onDocumentMouseMove, true);
|
||||||
|
this.staleHoverCheckActive = true;
|
||||||
|
}
|
||||||
|
|
||||||
|
private stopStaleHoverCheck(): void {
|
||||||
|
if (!this.staleHoverCheckActive) return;
|
||||||
|
document.removeEventListener("mousemove", this.onDocumentMouseMove, true);
|
||||||
|
this.staleHoverCheckActive = false;
|
||||||
|
}
|
||||||
|
|
||||||
|
private readonly onDocumentMouseMove = (): void => {
|
||||||
|
if (this.state.hover && !(this.ref.current?.matches(":hover") ?? false)) {
|
||||||
|
this.setState({ hover: false });
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
private readonly onMouseEnter = (): void => {
|
||||||
|
this.setState({ hover: true });
|
||||||
|
};
|
||||||
|
|
||||||
|
private readonly onMouseLeave = (): void => {
|
||||||
|
this.setState({ hover: false });
|
||||||
|
};
|
||||||
|
|
||||||
private readonly onFocusWithin = (event: FocusEvent<HTMLElement>): void => {
|
private readonly onFocusWithin = (event: FocusEvent<HTMLElement>): void => {
|
||||||
// Show the action toolbar for keyboard-visible focus, with what-input as a fallback signal.
|
// Show the action toolbar for keyboard-visible focus, with what-input as a fallback signal.
|
||||||
const target = event.target as HTMLElement;
|
const target = event.target as HTMLElement;
|
||||||
@@ -1321,8 +1368,8 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
|
|||||||
"data-layout": this.props.layout,
|
"data-layout": this.props.layout,
|
||||||
"data-self": isOwnEvent,
|
"data-self": isOwnEvent,
|
||||||
"data-event-id": this.props.mxEvent.getId(),
|
"data-event-id": this.props.mxEvent.getId(),
|
||||||
"onMouseEnter": () => this.setState({ hover: true }),
|
"onMouseEnter": this.onMouseEnter,
|
||||||
"onMouseLeave": () => this.setState({ hover: false }),
|
"onMouseLeave": this.onMouseLeave,
|
||||||
"onFocus": this.onFocusWithin,
|
"onFocus": this.onFocusWithin,
|
||||||
"onBlur": this.onBlurWithin,
|
"onBlur": this.onBlurWithin,
|
||||||
},
|
},
|
||||||
@@ -1384,8 +1431,8 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
|
|||||||
"data-shape": this.context.timelineRenderingType,
|
"data-shape": this.context.timelineRenderingType,
|
||||||
"data-self": isOwnEvent,
|
"data-self": isOwnEvent,
|
||||||
"data-has-reply": !!replyChain,
|
"data-has-reply": !!replyChain,
|
||||||
"onMouseEnter": () => this.setState({ hover: true }),
|
"onMouseEnter": this.onMouseEnter,
|
||||||
"onMouseLeave": () => this.setState({ hover: false }),
|
"onMouseLeave": this.onMouseLeave,
|
||||||
"onFocus": this.onFocusWithin,
|
"onFocus": this.onFocusWithin,
|
||||||
"onBlur": this.onBlurWithin,
|
"onBlur": this.onBlurWithin,
|
||||||
"onClick": (ev: MouseEvent) => {
|
"onClick": (ev: MouseEvent) => {
|
||||||
@@ -1517,8 +1564,8 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
|
|||||||
"data-self": isOwnEvent,
|
"data-self": isOwnEvent,
|
||||||
"data-event-id": this.props.mxEvent.getId(),
|
"data-event-id": this.props.mxEvent.getId(),
|
||||||
"data-has-reply": !!replyChain,
|
"data-has-reply": !!replyChain,
|
||||||
"onMouseEnter": () => this.setState({ hover: true }),
|
"onMouseEnter": this.onMouseEnter,
|
||||||
"onMouseLeave": () => this.setState({ hover: false }),
|
"onMouseLeave": this.onMouseLeave,
|
||||||
"onFocus": this.onFocusWithin,
|
"onFocus": this.onFocusWithin,
|
||||||
"onBlur": this.onBlurWithin,
|
"onBlur": this.onBlurWithin,
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -955,7 +955,7 @@ describe("RoomView", () => {
|
|||||||
expect(searchResultTile).not.toBeNull();
|
expect(searchResultTile).not.toBeNull();
|
||||||
|
|
||||||
await userEvent.hover(searchResultTile!);
|
await userEvent.hover(searchResultTile!);
|
||||||
await userEvent.click(await findByLabelText("Edit"));
|
await userEvent.click(await findByLabelText("Edit"), { skipHover: true });
|
||||||
|
|
||||||
await waitFor(() => {
|
await waitFor(() => {
|
||||||
expect(container.querySelector(".mx_RoomView_searchResultsPanel")).not.toBeInTheDocument();
|
expect(container.querySelector(".mx_RoomView_searchResultsPanel")).not.toBeInTheDocument();
|
||||||
@@ -1024,7 +1024,7 @@ describe("RoomView", () => {
|
|||||||
expect(searchResultTile).not.toBeNull();
|
expect(searchResultTile).not.toBeNull();
|
||||||
|
|
||||||
await userEvent.hover(searchResultTile!);
|
await userEvent.hover(searchResultTile!);
|
||||||
await userEvent.click(await findByLabelText("Edit"));
|
await userEvent.click(await findByLabelText("Edit"), { skipHover: true });
|
||||||
|
|
||||||
await expect(prom).resolves.toEqual(expect.objectContaining({ room_id: room2.roomId }));
|
await expect(prom).resolves.toEqual(expect.objectContaining({ room_id: room2.roomId }));
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -46,6 +46,7 @@ import PinningUtils from "../../../../../src/utils/PinningUtils";
|
|||||||
import { Layout } from "../../../../../src/settings/enums/Layout";
|
import { Layout } from "../../../../../src/settings/enums/Layout";
|
||||||
import { ScopedRoomContextProvider } from "../../../../../src/contexts/ScopedRoomContext.tsx";
|
import { ScopedRoomContextProvider } from "../../../../../src/contexts/ScopedRoomContext.tsx";
|
||||||
import SettingsStore from "../../../../../src/settings/SettingsStore";
|
import SettingsStore from "../../../../../src/settings/SettingsStore";
|
||||||
|
import EditorStateTransfer from "../../../../../src/utils/EditorStateTransfer";
|
||||||
import { RoomPermalinkCreator } from "../../../../../src/utils/permalinks/Permalinks";
|
import { RoomPermalinkCreator } from "../../../../../src/utils/permalinks/Permalinks";
|
||||||
import PlatformPeg from "../../../../../src/PlatformPeg";
|
import PlatformPeg from "../../../../../src/PlatformPeg";
|
||||||
|
|
||||||
@@ -159,6 +160,29 @@ describe("EventTile", () => {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function WrappedEventTiles(props: { events: MatrixEvent[]; editEvent?: MatrixEvent }) {
|
||||||
|
const roomContext = getRoomContext(room, {
|
||||||
|
timelineRenderingType: TimelineRenderingType.Room,
|
||||||
|
});
|
||||||
|
|
||||||
|
return (
|
||||||
|
<MatrixClientContext.Provider value={client}>
|
||||||
|
<ScopedRoomContextProvider {...roomContext}>
|
||||||
|
{props.events.map((event) => (
|
||||||
|
<EventTile
|
||||||
|
key={event.getId()}
|
||||||
|
mxEvent={event}
|
||||||
|
replacingEventId={event.replacingEventId()}
|
||||||
|
editState={
|
||||||
|
props.editEvent?.getId() === event.getId() ? new EditorStateTransfer(event) : undefined
|
||||||
|
}
|
||||||
|
/>
|
||||||
|
))}
|
||||||
|
</ScopedRoomContextProvider>
|
||||||
|
</MatrixClientContext.Provider>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
jest.clearAllMocks();
|
jest.clearAllMocks();
|
||||||
|
|
||||||
@@ -389,7 +413,9 @@ describe("EventTile", () => {
|
|||||||
|
|
||||||
expect(container.querySelector(".mx_MessageTimestamp")).toBeNull();
|
expect(container.querySelector(".mx_MessageTimestamp")).toBeNull();
|
||||||
|
|
||||||
fireEvent.focus(getTile(container));
|
act(() => {
|
||||||
|
getTile(container).focus();
|
||||||
|
});
|
||||||
|
|
||||||
expect(container.querySelector(".mx_MessageTimestamp")).not.toBeNull();
|
expect(container.querySelector(".mx_MessageTimestamp")).not.toBeNull();
|
||||||
});
|
});
|
||||||
@@ -613,7 +639,9 @@ describe("EventTile", () => {
|
|||||||
});
|
});
|
||||||
const { container } = getComponent();
|
const { container } = getComponent();
|
||||||
|
|
||||||
fireEvent.focus(getTile(container));
|
act(() => {
|
||||||
|
getTile(container).focus();
|
||||||
|
});
|
||||||
|
|
||||||
expect(container.querySelector(".mx_MessageActionBar")).not.toBeNull();
|
expect(container.querySelector(".mx_MessageActionBar")).not.toBeNull();
|
||||||
});
|
});
|
||||||
@@ -627,10 +655,14 @@ describe("EventTile", () => {
|
|||||||
const { container } = getComponent();
|
const { container } = getComponent();
|
||||||
const tile = getTile(container);
|
const tile = getTile(container);
|
||||||
|
|
||||||
fireEvent.focus(tile);
|
act(() => {
|
||||||
|
tile.focus();
|
||||||
|
});
|
||||||
expect(container.querySelector(".mx_MessageActionBar")).not.toBeNull();
|
expect(container.querySelector(".mx_MessageActionBar")).not.toBeNull();
|
||||||
|
|
||||||
fireEvent.blur(tile);
|
act(() => {
|
||||||
|
tile.blur();
|
||||||
|
});
|
||||||
|
|
||||||
expect(container.querySelector(".mx_MessageActionBar")).toBeNull();
|
expect(container.querySelector(".mx_MessageActionBar")).toBeNull();
|
||||||
});
|
});
|
||||||
@@ -1366,6 +1398,48 @@ describe("EventTile", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("does not leave a stale message action bar when switching edited events", async () => {
|
||||||
|
const firstEvent = mkMessage({
|
||||||
|
room: room.roomId,
|
||||||
|
user: "@alice:example.org",
|
||||||
|
msg: "First message",
|
||||||
|
event: true,
|
||||||
|
});
|
||||||
|
const secondEvent = mkMessage({
|
||||||
|
room: room.roomId,
|
||||||
|
user: "@alice:example.org",
|
||||||
|
msg: "Second message",
|
||||||
|
event: true,
|
||||||
|
});
|
||||||
|
const events = [firstEvent, secondEvent];
|
||||||
|
|
||||||
|
const matches = jest.spyOn(HTMLElement.prototype, "matches").mockImplementation(function (
|
||||||
|
this: HTMLElement,
|
||||||
|
selector: string,
|
||||||
|
) {
|
||||||
|
if (selector === ":focus-visible") {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
return Element.prototype.matches.call(this, selector);
|
||||||
|
});
|
||||||
|
|
||||||
|
const { container, rerender } = render(<WrappedEventTiles events={events} editEvent={firstEvent} />);
|
||||||
|
const editingTile = container.querySelector(".mx_EventTile_isEditing");
|
||||||
|
|
||||||
|
expect(editingTile).not.toBeNull();
|
||||||
|
fireEvent.focusIn(editingTile!);
|
||||||
|
expect(container.querySelectorAll(".mx_MessageActionBar")).toHaveLength(0);
|
||||||
|
|
||||||
|
rerender(<WrappedEventTiles events={events} editEvent={secondEvent} />);
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(container.querySelectorAll(".mx_EventTile_isEditing")).toHaveLength(1);
|
||||||
|
expect(container.querySelectorAll(".mx_MessageActionBar")).toHaveLength(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
matches.mockRestore();
|
||||||
|
});
|
||||||
|
|
||||||
it("should display the not encrypted status for an unencrypted event when the room becomes encrypted", async () => {
|
it("should display the not encrypted status for an unencrypted event when the room becomes encrypted", async () => {
|
||||||
jest.spyOn(client.getCrypto()!, "getEncryptionInfoForEvent").mockResolvedValue({
|
jest.spyOn(client.getCrypto()!, "getEncryptionInfoForEvent").mockResolvedValue({
|
||||||
shieldColour: EventShieldColour.NONE,
|
shieldColour: EventShieldColour.NONE,
|
||||||
|
|||||||
Reference in New Issue
Block a user