From ce19ab570a752b10c3abd1234d6262c99d97392b Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Thu, 4 Jul 2024 14:57:44 +0200 Subject: [PATCH 1/7] feat: reworked the layout switcher --- res/css/views/settings/_LayoutSwitcher.pcss | 119 ++++---- .../views/settings/LayoutSwitcher.tsx | 267 ++++++++++-------- src/i18n/strings/en_EN.json | 4 +- 3 files changed, 214 insertions(+), 176 deletions(-) diff --git a/res/css/views/settings/_LayoutSwitcher.pcss b/res/css/views/settings/_LayoutSwitcher.pcss index 571b9a1cf1c..a88051bfd82 100644 --- a/res/css/views/settings/_LayoutSwitcher.pcss +++ b/res/css/views/settings/_LayoutSwitcher.pcss @@ -15,79 +15,76 @@ See the License for the specific language governing permissions and limitations under the License. */ -.mx_LayoutSwitcher_RadioButtons { +.mx_LayoutSwitcher_LayoutSelector { display: flex; - flex-direction: row; - gap: 24px; - width: 100%; + flex-direction: column; + gap: var(--cpd-space-4x) !important; - color: $primary-content; + .mxLayoutSwitcher_LayoutSelector_LayoutRadio { + border: 1px solid var(--cpd-color-border-interactive-primary); + border-radius: var(--cpd-space-2x); - > .mx_LayoutSwitcher_RadioButton { - flex-grow: 0; - flex-shrink: 1; - display: flex; - flex-direction: column; - overflow: hidden; - - flex-basis: 33%; - min-width: 0; - - border: 1px solid $quinary-content; - border-radius: 10px; - - .mx_EventTile_msgOption, - .mx_MessageActionBar { - display: none; - } - - .mx_LayoutSwitcher_RadioButton_preview { - flex-grow: 1; + .mxLayoutSwitcher_LayoutSelector_LayoutRadio_inline { display: flex; + /* + * 10px + */ + gap: calc(var(--cpd-space-2x) + var(--cpd-space-0-5x)); align-items: center; - padding: 10px; - pointer-events: none; - - .mx_EventTile[data-layout="bubble"] .mx_EventTile_line { - padding-right: 11px; - } - } - - .mx_StyledRadioButton { - flex-grow: 0; - padding: 10px; } - .mx_EventTile_content { - margin-right: 0; + .mxLayoutSwitcher_LayoutSelector_LayoutRadio_inline, + .mxLayoutSwitcher_LayoutSelector_LayoutRadio_EventTilePreview { + margin: var(--cpd-space-3x); } - &.mx_LayoutSwitcher_RadioButton_selected { - border-color: var(--cpd-color-bg-accent-rest); - } - } - - .mx_StyledRadioButton { - border-top: 1px solid $quinary-content; - } - - .mx_StyledRadioButton_checked { - background-color: var(--cpd-color-bg-subtle-secondary); - } + /** + * Override the event tile style to make it fit in the selector + * Tweak also hover style and remove action bar + */ + .mxLayoutSwitcher_LayoutSelector_LayoutRadio_EventTilePreview { + pointer-events: none; - .mx_EventTile { - margin: 0; - &[data-layout="bubble"] { - margin-right: 40px; - flex-shrink: 1; - } - &[data-layout="irc"] { - > a { - display: none; + .mx_EventTile { + margin: 0; + + /** + * Hide the message options and message action bar in the preview + */ + .mx_EventTile_msgOption, + .mx_MessageActionBar { + display: none; + } + + .mx_EventTile_content { + margin-right: 0; + } + + &[data-layout="group"] { + margin-top: calc(var(--cpd-space-3x) * -1); + } + + /** + * Add margin to center the bubble + */ + &[data-layout="bubble"] { + /** + * Add the layout margin and the margin to vertically center the bubble + */ + margin-top: var(--cpd-space-6x); + margin-right: 34px; + flex-shrink: 1; + } + + .mx_EventTile_line { + max-width: 100%; + } } } - .mx_EventTile_line { - max-width: 90%; + + .mxLayoutSwitcher_LayoutSelector_LayoutRadio_separator { + border-top: 0; + border-bottom: 1px solid var(--cpd-color-border-interactive-secondary); } } } diff --git a/src/components/views/settings/LayoutSwitcher.tsx b/src/components/views/settings/LayoutSwitcher.tsx index 101a75fbe89..de3827b63df 100644 --- a/src/components/views/settings/LayoutSwitcher.tsx +++ b/src/components/views/settings/LayoutSwitcher.tsx @@ -1,131 +1,170 @@ /* -Copyright 2019 New Vector Ltd -Copyright 2019 - 2021 The Matrix.org Foundation C.I.C. -Copyright 2021 Šimon Brandner + * Copyright 2024 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. + */ -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 +import React, { JSX, useEffect, useState } from "react"; +import { Field, HelpMessage, InlineField, Label, RadioControl, Root, ToggleControl } from "@vector-im/compound-web"; - 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"; -import classNames from "classnames"; - -import SettingsStore from "../../../settings/SettingsStore"; -import EventTilePreview from "../elements/EventTilePreview"; -import StyledRadioButton from "../elements/StyledRadioButton"; +import SettingsSubsection from "./shared/SettingsSubsection"; import { _t } from "../../../languageHandler"; -import { Layout } from "../../../settings/enums/Layout"; +import SettingsStore from "../../../settings/SettingsStore"; import { SettingLevel } from "../../../settings/SettingLevel"; -import SettingsSubsection from "./shared/SettingsSubsection"; +import { useSettingValue } from "../../../hooks/useSettings"; +import { Layout } from "../../../settings/enums/Layout"; +import EventTilePreview from "../elements/EventTilePreview"; +import { useMatrixClientContext } from "../../../contexts/MatrixClientContext"; -interface IProps { - userId?: string; - displayName?: string; - avatarUrl?: string; - messagePreviewText: string; - onLayoutChanged: (layout: Layout) => void; +/** + * A section to switch between different message layouts. + */ +export function LayoutSwitcher(): JSX.Element { + return ( + + + + + ); } -interface IState { +/** + * A selector to choose the layout of the messages. + */ +function LayoutSelector(): JSX.Element { + return ( + { + // We don't have any file in the form, we can cast it as string safely + const newLayout = new FormData(evt.currentTarget).get("layout") as string | null; + await SettingsStore.setValue("layout", null, SettingLevel.DEVICE, newLayout); + }} + > + + + + + ); +} + +/** + * A radio button to select a layout. + */ +interface LayoutRadioProps { + /** + * The value of the layout. + */ layout: Layout; + /** + * The label to display for the layout. + */ + label: string; } -export default class LayoutSwitcher extends React.Component { - public constructor(props: IProps) { - super(props); +/** + * A radio button to select a layout. + * @param layout + * @param label + */ +function LayoutRadio({ layout, label }: LayoutRadioProps): JSX.Element { + const currentLayout = useSettingValue("layout"); + const eventTileInfo = useEventTileInfo(); - this.state = { - layout: SettingsStore.getValue("layout"), - }; - } + return ( + + + + ); +} + +type EventTileInfo = { + /** + * The ID of the user to display. + */ + userId: string; + /** + * The display name of the user to display. + */ + displayName?: string; + /** + * The avatar URL of the user to display. + */ + avatarUrl?: string; +}; + +/** + * Fetch the information to display in the event tile preview. + */ +function useEventTileInfo(): EventTileInfo { + const matrixClient = useMatrixClientContext(); + const userId = matrixClient.getSafeUserId(); + const [eventTileInfo, setEventTileInfo] = useState({ userId }); - private onLayoutChange = (e: React.ChangeEvent): void => { - const layout = e.target.value as Layout; + useEffect(() => { + const run = async (): Promise => { + const profileInfo = await matrixClient.getProfileInfo(userId); + setEventTileInfo({ + userId, + displayName: profileInfo.displayname, + avatarUrl: profileInfo.avatar_url, + }); + }; - this.setState({ layout: layout }); - SettingsStore.setValue("layout", null, SettingLevel.DEVICE, layout); - this.props.onLayoutChanged(layout); - }; + run(); + }, [userId, matrixClient, setEventTileInfo]); + return eventTileInfo; +} - public render(): React.ReactNode { - const ircClasses = classNames("mx_LayoutSwitcher_RadioButton", { - mx_LayoutSwitcher_RadioButton_selected: this.state.layout == Layout.IRC, - }); - const groupClasses = classNames("mx_LayoutSwitcher_RadioButton", { - mx_LayoutSwitcher_RadioButton_selected: this.state.layout == Layout.Group, - }); - const bubbleClasses = classNames("mx_LayoutSwitcher_RadioButton", { - mx_LayoutSwitcher_RadioButton_selected: this.state.layout === Layout.Bubble, - }); +/** + * A toggleable setting to enable or disable the compact layout. + */ +function ToggleCompactLayout(): JSX.Element { + const compactLayoutEnabled = useSettingValue("useCompactLayout"); + const layout = useSettingValue("layout"); - return ( - -
- - - -
- - ); - } + return ( + { + const checked = new FormData(evt.currentTarget).get("compactLayout") === "on"; + await SettingsStore.setValue("useCompactLayout", null, SettingLevel.DEVICE, checked); + }} + > + + } + > + + {_t("settings|appearance|compact_layout_description")} + + + ); } diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 5a532d00789..5272e22c855 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -2416,6 +2416,8 @@ "always_show_message_timestamps": "Always show message timestamps", "appearance": { "bundled_emoji_font": "Use bundled emoji font", + "compact_layout": "Show compact text and messages", + "compact_layout_description": "Modern layout must be selected to use this feature.", "custom_font": "Use a system font", "custom_font_description": "Set the name of a font installed on your system & %(brand)s will attempt to use it.", "custom_font_name": "System font name", @@ -2432,7 +2434,7 @@ "image_size_default": "Default", "image_size_large": "Large", "layout_bubbles": "Message bubbles", - "layout_irc": "IRC (Experimental)", + "layout_irc": "IRC (experimental)", "match_system_theme": "Match system theme", "timeline_image_size": "Image size in the timeline" }, From b961d73bb5aa41ff9134bf78a74c83f91fd8e513 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Thu, 4 Jul 2024 15:39:53 +0200 Subject: [PATCH 2/7] feat: make the classname optional in EventTilePreview.tsx --- src/components/views/elements/EventTilePreview.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/elements/EventTilePreview.tsx b/src/components/views/elements/EventTilePreview.tsx index fd11d372dc3..4b8ccb0ed90 100644 --- a/src/components/views/elements/EventTilePreview.tsx +++ b/src/components/views/elements/EventTilePreview.tsx @@ -37,7 +37,7 @@ interface IProps { /** * classnames to apply to the wrapper of the preview */ - className: string; + className?: string; /** * The ID of the displayed user From daba3df9003665a56df2f8a7cd922b47d74fa83c Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Thu, 4 Jul 2024 15:33:10 +0200 Subject: [PATCH 3/7] test: add tests to LayoutSwitcher --- .../views/settings/LayoutSwitcher-test.tsx | 97 ++++ .../LayoutSwitcher-test.tsx.snap | 426 ++++++++++++++++++ 2 files changed, 523 insertions(+) create mode 100644 test/components/views/settings/LayoutSwitcher-test.tsx create mode 100644 test/components/views/settings/__snapshots__/LayoutSwitcher-test.tsx.snap diff --git a/test/components/views/settings/LayoutSwitcher-test.tsx b/test/components/views/settings/LayoutSwitcher-test.tsx new file mode 100644 index 00000000000..c5d8a455fc1 --- /dev/null +++ b/test/components/views/settings/LayoutSwitcher-test.tsx @@ -0,0 +1,97 @@ +/* + * Copyright 2024 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"; +import { act, render, screen, waitFor } from "@testing-library/react"; +import { mocked } from "jest-mock"; + +import { LayoutSwitcher } from "../../../../src/components/views/settings/LayoutSwitcher"; +import MatrixClientContext from "../../../../src/contexts/MatrixClientContext"; +import { stubClient } from "../../../test-utils"; +import SettingsStore from "../../../../src/settings/SettingsStore"; +import { SettingLevel } from "../../../../src/settings/SettingLevel"; +import { Layout } from "../../../../src/settings/enums/Layout"; + +describe("", () => { + const matrixClient = stubClient(); + const profileInfo = { + displayname: "Alice", + }; + + async function renderLayoutSwitcher() { + const renderResult = render( + + + , + ); + + // Wait for the profile info to be displayed in the event tile preview + // Also avoid act warning + await waitFor(() => expect(screen.getAllByText(profileInfo.displayname).length).toBe(3)); + return renderResult; + } + + beforeEach(async () => { + await SettingsStore.setValue("layout", null, SettingLevel.DEVICE, Layout.Group); + mocked(matrixClient).getProfileInfo.mockResolvedValue(profileInfo); + }); + + it("should render", async () => { + const { asFragment } = await renderLayoutSwitcher(); + expect(asFragment()).toMatchSnapshot(); + }); + + describe("layout selection", () => { + it("should display the modern layout", async () => { + await renderLayoutSwitcher(); + expect(screen.getByRole("radio", { name: "Modern" })).toBeChecked(); + }); + + it("should change the layout when selected", async () => { + await renderLayoutSwitcher(); + act(() => screen.getByRole("radio", { name: "Message bubbles" }).click()); + + expect(screen.getByRole("radio", { name: "Message bubbles" })).toBeChecked(); + await waitFor(() => expect(SettingsStore.getValue("layout")).toBe(Layout.Bubble)); + }); + }); + + describe("compact layout", () => { + beforeEach(async () => { + await SettingsStore.setValue("useCompactLayout", null, SettingLevel.DEVICE, false); + }); + + it("should be enabled", async () => { + await SettingsStore.setValue("useCompactLayout", null, SettingLevel.DEVICE, true); + await renderLayoutSwitcher(); + + expect(screen.getByRole("checkbox", { name: "Show compact text and messages" })).toBeChecked(); + }); + + it("should change the setting when toggled", async () => { + await renderLayoutSwitcher(); + act(() => screen.getByRole("checkbox", { name: "Show compact text and messages" }).click()); + + await waitFor(() => expect(SettingsStore.getValue("useCompactLayout")).toBe(true)); + }); + + it("should be disabled when the modern layout is not enabled", async () => { + await SettingsStore.setValue("layout", null, SettingLevel.DEVICE, Layout.Bubble); + await renderLayoutSwitcher(); + expect(screen.getByRole("checkbox", { name: "Show compact text and messages" })).toBeDisabled(); + }); + }); +}); diff --git a/test/components/views/settings/__snapshots__/LayoutSwitcher-test.tsx.snap b/test/components/views/settings/__snapshots__/LayoutSwitcher-test.tsx.snap new file mode 100644 index 00000000000..fef33c497bf --- /dev/null +++ b/test/components/views/settings/__snapshots__/LayoutSwitcher-test.tsx.snap @@ -0,0 +1,426 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` should render 1`] = ` + +
+
+

+ Message layout +

+
+
+
+
+