Room list: make the filter resize correctly (#30788) (#30795)

* fix: make the filter resize correctly

* test: update snapshot

(cherry picked from commit 155c0b6a0c)

Co-authored-by: Florian Duros <florianduros@element.io>

package.json

@@ -134,7 +134,7 @@
         "maplibre-gl": "^5.0.0",
         "matrix-encrypt-attachment": "^1.0.3",
         "matrix-events-sdk": "0.0.1",
-        "matrix-js-sdk": "github:matrix-org/matrix-js-sdk#develop",
+        "matrix-js-sdk": "38.3.0-rc.0",
         "matrix-widget-api": "^1.10.0",
         "memoize-one": "^6.0.0",
         "mime": "^4.0.4",

res/css/views/rooms/RoomListPanel/_RoomListPrimaryFilters.pcss

@@ -12,10 +12,7 @@
         display: none;
     }
 
-    ul {
-        margin: unset;
-        padding: unset;
-        list-style-type: none;
+    .mx_RoomListPrimaryFilters_list {
         /**
          * The InteractionObserver needs the height to be set to work properly.
          */

src/components/views/rooms/RoomListPanel/RoomListPrimaryFilters.tsx

@@ -59,6 +59,7 @@ export function RoomListPrimaryFilters({ vm }: RoomListPrimaryFiltersProps): JSX
                 align="center"
                 gap="var(--cpd-space-2x)"
                 wrap="wrap"
+                className="mx_RoomListPrimaryFilters_list"
                 ref={ref}
             >
                 {filters.map((filter, i) => (
@@ -103,7 +104,7 @@ function useCollapseFilters<T extends HTMLElement>(
 
                 // If the previous element is on the left element of the current one, it means that the filter is wrapping
                 const previousSibling = child.previousElementSibling as HTMLElement | null;
-                if (previousSibling && child.offsetLeft < previousSibling.offsetLeft) {
+                if (previousSibling && child.offsetLeft <= previousSibling.offsetLeft) {
                     if (!isWrapping) setWrappingIndex(i);
                     isWrapping = true;
                 }

test/unit-tests/components/views/rooms/RoomListPanel/RoomListPrimaryFilters-test.tsx

@@ -35,15 +35,26 @@ describe("<RoomListPrimaryFilters />", () => {
 
         vm = {
             primaryFilters: [
-                { name: "People", active: false, toggle: filterToggleMocks[0], key: FilterKey.PeopleFilter },
-                { name: "Rooms", active: true, toggle: filterToggleMocks[1], key: FilterKey.RoomsFilter },
+                { name: "People", active: true, toggle: filterToggleMocks[0], key: FilterKey.PeopleFilter },
+                { name: "Rooms", active: false, toggle: filterToggleMocks[1], key: FilterKey.RoomsFilter },
                 { name: "Unreads", active: false, toggle: filterToggleMocks[2], key: FilterKey.UnreadFilter },
             ],
         } as unknown as RoomListViewState;
     });
 
+    function mockFiltersOffsetLeft() {
+        // Use `getByText` instead of `getByRole` to bypass the aria-hidden
+        jest.spyOn(screen.getByText("People"), "offsetLeft", "get").mockReturnValue(0);
+        jest.spyOn(screen.getByText("Rooms"), "offsetLeft", "get").mockReturnValue(30);
+        jest.spyOn(screen.getByText("Unreads"), "offsetLeft", "get").mockReturnValue(60);
+
+        // @ts-ignore
+        act(() => resizeCallback([{ target: screen.getByRole("listbox", { name: "Room list filters" }) }]));
+    }
+
     it("should renders all filters correctly", () => {
         const { asFragment } = render(<RoomListPrimaryFilters vm={vm} />);
+        mockFiltersOffsetLeft();
 
         // Check that all filters are rendered
         expect(screen.getByRole("option", { name: "People" })).toBeInTheDocument();
@@ -51,8 +62,8 @@ describe("<RoomListPrimaryFilters />", () => {
         expect(screen.getByRole("option", { name: "Unreads" })).toBeInTheDocument();
 
         // Check that the active filter is marked as selected
-        expect(screen.getByRole("option", { name: "Rooms" })).toHaveAttribute("aria-selected", "true");
-        expect(screen.getByRole("option", { name: "People" })).toHaveAttribute("aria-selected", "false");
+        expect(screen.getByRole("option", { name: "People" })).toHaveAttribute("aria-selected", "true");
+        expect(screen.getByRole("option", { name: "Rooms" })).toHaveAttribute("aria-selected", "false");
         expect(screen.getByRole("option", { name: "Unreads" })).toHaveAttribute("aria-selected", "false");
 
         expect(asFragment()).toMatchSnapshot();
@@ -61,6 +72,7 @@ describe("<RoomListPrimaryFilters />", () => {
     it("should call toggle function when a filter is clicked", async () => {
         const user = userEvent.setup();
         render(<RoomListPrimaryFilters vm={vm} />);
+        mockFiltersOffsetLeft();
 
         // Click on an inactive filter
         await user.click(screen.getByRole("option", { name: "People" }));
@@ -69,24 +81,27 @@ describe("<RoomListPrimaryFilters />", () => {
         expect(filterToggleMocks[0]).toHaveBeenCalledTimes(1);
     });
 
-    function mockFiltersOffsetLeft() {
-        jest.spyOn(screen.getByRole("option", { name: "People" }), "offsetLeft", "get").mockReturnValue(0);
-        jest.spyOn(screen.getByRole("option", { name: "Rooms" }), "offsetLeft", "get").mockReturnValue(30);
+    function makeUnreadWrapping() {
+        // Use `getByText` instead of `getByRole` to bypass the aria-hidden
+        jest.spyOn(screen.getByText("People"), "offsetLeft", "get").mockReturnValue(0);
+        jest.spyOn(screen.getByText("Rooms"), "offsetLeft", "get").mockReturnValue(30);
         // Unreads is wrapping
-        jest.spyOn(screen.getByRole("option", { name: "Unreads" }), "offsetLeft", "get").mockReturnValue(0);
+        jest.spyOn(screen.getByText("Unreads"), "offsetLeft", "get").mockReturnValue(0);
+
+        // @ts-ignore
+        act(() => resizeCallback([{ target: screen.getByRole("listbox", { name: "Room list filters" }) }]));
     }
 
     it("should hide or display filters if they are wrapping", async () => {
         const user = userEvent.setup();
         render(<RoomListPrimaryFilters vm={vm} />);
+        mockFiltersOffsetLeft();
 
         // No filter is wrapping, so chevron shouldn't be visible
         expect(screen.queryByRole("button", { name: "Expand filter list" })).toBeNull();
         expect(screen.queryByRole("option", { name: "Unreads" })).toBeVisible();
 
-        mockFiltersOffsetLeft();
-        // @ts-ignore
-        act(() => resizeCallback([{ target: screen.getByRole("listbox", { name: "Room list filters" }) }]));
+        makeUnreadWrapping();
 
         // The Unreads filter is wrapping, it should not be visible
         expect(screen.queryByRole("option", { name: "Unreads" })).toBeNull();
@@ -107,9 +122,7 @@ describe("<RoomListPrimaryFilters />", () => {
 
         const user = userEvent.setup();
         render(<RoomListPrimaryFilters vm={vm} />);
-        mockFiltersOffsetLeft();
-        // @ts-ignore
-        act(() => resizeCallback([{ target: screen.getByRole("listbox", { name: "Room list filters" }) }]));
+        makeUnreadWrapping();
 
         // Unread filter should be moved to the first position
         expect(screen.getByRole("listbox", { name: "Room list filters" }).children[0]).toBe(
@@ -122,4 +135,21 @@ describe("<RoomListPrimaryFilters />", () => {
             screen.getByRole("option", { name: "Unreads" }),
         );
     });
+
+    it("should hide the filter is the previous is on the same vertical position", async () => {
+        render(<RoomListPrimaryFilters vm={vm} />);
+        mockFiltersOffsetLeft();
+
+        jest.spyOn(screen.getByRole("option", { name: "People" }), "offsetLeft", "get").mockReturnValue(0);
+        // Rooms is wrapping
+        jest.spyOn(screen.getByRole("option", { name: "Rooms" }), "offsetLeft", "get").mockReturnValue(0);
+
+        // @ts-ignore
+        act(() => resizeCallback([{ target: screen.getByRole("listbox", { name: "Room list filters" }) }]));
+
+        // The Unreads filter is wrapping, it should not be visible
+        expect(screen.queryByRole("option", { name: "Rooms" })).toBeNull();
+        // Now filters are wrapping, so chevron should be visible
+        expect(screen.getByRole("button", { name: "Expand filter list" })).toBeVisible();
+    });
 });

test/unit-tests/components/views/rooms/RoomListPanel/__snapshots__/RoomListPrimaryFilters-test.tsx.snap

@@ -9,14 +9,14 @@ exports[`<RoomListPrimaryFilters /> should renders all filters correctly 1`] = `
   >
     <div
       aria-label="Room list filters"
-      class="flex"
+      class="flex mx_RoomListPrimaryFilters_list"
       id="«r0»"
       role="listbox"
       style="--mx-flex-display: flex; --mx-flex-direction: row; --mx-flex-align: center; --mx-flex-justify: start; --mx-flex-gap: var(--cpd-space-2x); --mx-flex-wrap: wrap;"
     >
       <button
         aria-hidden="false"
-        aria-selected="false"
+        aria-selected="true"
         class="_chat-filter_5qdp0_8"
         role="option"
         tabindex="0"
@@ -25,7 +25,7 @@ exports[`<RoomListPrimaryFilters /> should renders all filters correctly 1`] = `
       </button>
       <button
         aria-hidden="false"
-        aria-selected="true"
+        aria-selected="false"
         class="_chat-filter_5qdp0_8"
         role="option"
         tabindex="0"

yarn.lock

@@ -2652,10 +2652,10 @@
     emojibase "^15.3.1"
     emojibase-data "^15.3.1"
 
-"@matrix-org/matrix-sdk-crypto-wasm@^15.1.0":
-  version "15.1.0"
-  resolved "https://registry.yarnpkg.com/@matrix-org/matrix-sdk-crypto-wasm/-/matrix-sdk-crypto-wasm-15.1.0.tgz#653956f5f6daced55a9df3d2c1114eb2c017b528"
-  integrity sha512-ZsDdjn46J3+VxsDLmaSODuS+qtGZB/i3Cg9tWL1QPNjvAWzNaTHQ7glleByI2PKVBm83aklfuhGKT2MqE1ZsEA==
+"@matrix-org/matrix-sdk-crypto-wasm@^15.2.0":
+  version "15.3.0"
+  resolved "https://registry.yarnpkg.com/@matrix-org/matrix-sdk-crypto-wasm/-/matrix-sdk-crypto-wasm-15.3.0.tgz#141fd041ae382b793369bcee4394b0b577bdea0c"
+  integrity sha512-QyxHvncvkl7nf+tnn92PjQ54gMNV8hMSpiukiDgNrqF6IYwgySTlcSdkPYdw8QjZJ0NR6fnVrNzMec0OohM3wA==
 
 "@matrix-org/react-sdk-module-api@^2.4.0":
   version "2.5.0"
@@ -4735,7 +4735,7 @@
     classnames "^2.5.1"
     vaul "^1.0.0"
 
-"@vector-im/matrix-wysiwyg-wasm@link:../../Library/Caches/Yarn/v6/npm-@vector-im-matrix-wysiwyg-2.39.0-a6238e517f23a2f3025d9c65445914771c63b163-integrity/node_modules/bindings/wysiwyg-wasm":
+"@vector-im/matrix-wysiwyg-wasm@link:../../../.cache/yarn/v6/npm-@vector-im-matrix-wysiwyg-2.39.0-a6238e517f23a2f3025d9c65445914771c63b163-integrity/node_modules/bindings/wysiwyg-wasm":
   version "0.0.0"
   uid ""
 
@@ -11040,12 +11040,13 @@ matrix-events-sdk@0.0.1:
   resolved "https://registry.yarnpkg.com/matrix-events-sdk/-/matrix-events-sdk-0.0.1.tgz#c8c38911e2cb29023b0bbac8d6f32e0de2c957dd"
   integrity sha512-1QEOsXO+bhyCroIe2/A5OwaxHvBm7EsSQ46DEDn8RBIfQwN5HWBpFvyWWR4QY0KHPPnnJdI99wgRiAl7Ad5qaA==
 
-"matrix-js-sdk@github:matrix-org/matrix-js-sdk#develop":
-  version "38.1.0"
-  resolved "https://codeload.github.com/matrix-org/matrix-js-sdk/tar.gz/82aa04d894d7adc7c6be245220c4a89b69357d26"
+matrix-js-sdk@38.3.0-rc.0:
+  version "38.3.0-rc.0"
+  resolved "https://registry.yarnpkg.com/matrix-js-sdk/-/matrix-js-sdk-38.3.0-rc.0.tgz#fca926b3495896383d3ba9eb50204dc7359b1d4c"
+  integrity sha512-7q/xLXWSd4N7c3cymg9uqWgfq9cqnBMHR4/hgBDoNel3EBgK0RFOJuqSvoJCS9rwO7W0TMnjEc8ElnK5LmQFAg==
   dependencies:
     "@babel/runtime" "^7.12.5"
-    "@matrix-org/matrix-sdk-crypto-wasm" "^15.1.0"
+    "@matrix-org/matrix-sdk-crypto-wasm" "^15.2.0"
     another-json "^0.2.0"
     bs58 "^6.0.0"
     content-type "^1.0.4"