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

Prevent select element in General settings overflowing in a room with very long room-id #11597

Merged
merged 8 commits into from
Oct 9, 2023
Merged

Conversation

ABHIXIT2
Copy link
Contributor

@ABHIXIT2 ABHIXIT2 commented Sep 9, 2023

This PR solves this issue element-hq/element-web#25614

There were three issues:
-The General window in th room setting had a horizontal scroll
-The main address window width wasn't restricted(even after solving first issue it was overflowing)
-The address was overlaping with the button

element-testing-on-the-app-google-chrome-2023-09-09-10-03-39_q2XNQYXZ.mp4

I corrected the first issue and second issue by setting grid-template-columns: 1fr to grid-template-columns: minmax(0, 1fr).

Screenshot 2023-09-07 181005

I set the (text-overflow:ellipsis;) to remove the overlapping of address and the button.

Element.1._.Testing.1.-.Google.Chrome.2023-09-09.10-28-36.mp4

Here's what your changelog entry will look like:

🐛 Bug Fixes

  • Prevent select element in General settings overflowing in a room with very long room-id (#11597). Contributed by @ABHIXIT2.

@ABHIXIT2 ABHIXIT2 requested a review from a team as a code owner September 9, 2023 04:23
@github-actions github-actions bot added the Z-Community-PR Issue is solved by a community member's PR label Sep 9, 2023
@ABHIXIT2
Copy link
Contributor Author

@andybalaam please review this PR

@andybalaam andybalaam added the T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems label Sep 11, 2023
@andybalaam
Copy link
Member

@andybalaam please review this PR

@ABHIXIT2 there is no need to mention someone specifically - we regularly review new PRs and will get back to you. Thanks for your contribution!

Copy link
Contributor

@MidhunSureshR MidhunSureshR left a comment

Choose a reason for hiding this comment

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

Hello 👋, thanks for the PR!

res/css/views/elements/_Field.pcss Outdated Show resolved Hide resolved
@MidhunSureshR MidhunSureshR removed the request for review from andybalaam September 11, 2023 13:05
@ABHIXIT2
Copy link
Contributor Author

I've commited the changes as suggested and reversed done by me that were not useful.

Copy link
Contributor

@MidhunSureshR MidhunSureshR left a comment

Choose a reason for hiding this comment

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

You need to sign off this PR; you can just comment Signed-off-by: Your Name <your@email.example.org>.

@ABHIXIT2
Copy link
Contributor Author

Signed-off-by: Abhinav Dixit dabhinav378@gmail.com

@MidhunSureshR MidhunSureshR changed the title #25614 Updated mx_Field select #25614 Prevent select element in General settings from overflowing with a very long room-id Sep 14, 2023
@MidhunSureshR MidhunSureshR changed the title #25614 Prevent select element in General settings from overflowing with a very long room-id Prevent select element in General settings overflowing in a room with very long room-id Sep 14, 2023
@MidhunSureshR
Copy link
Contributor

Can you write a cypress test for this in https://github.com/matrix-org/matrix-react-sdk/blob/7a7e1fb43b49b994b1c901e9151125d0642f5a52/cypress/e2e/settings/general-room-settings-tab.spec.ts? You can assert that the width of the select element is less than that of .mx_Dialog.

@ABHIXIT2
Copy link
Contributor Author

Can you write a cypress test for this in https://github.com/matrix-org/matrix-react-sdk/blob/7a7e1fb43b49b994b1c901e9151125d0642f5a52/cypress/e2e/settings/general-room-settings-tab.spec.ts? You can assert that the width of the select element is less than that of .mx_Dialog.

Okay,I'll try

@ABHIXIT2
Copy link
Contributor Author

Can you write a cypress test for this in https://github.com/matrix-org/matrix-react-sdk/blob/7a7e1fb43b49b994b1c901e9151125d0642f5a52/cypress/e2e/settings/general-room-settings-tab.spec.ts? You can assert that the width of the select element is less than that of .mx_Dialog.

Okay,I'll try

I had been trying, not very familiar with the technology and also how it works here.
Can you please guide me, would be great help.

@andybalaam
Copy link
Member

I had been trying, not very familiar with the technology and also how it works here. Can you please guide me, would be great help.

There are several tests inside https://github.com/matrix-org/matrix-react-sdk/tree/develop/cypress/e2e/settings that might help you to design one. Do ask if you have specific questions.

There are some instructions at https://github.com/matrix-org/matrix-react-sdk/blob/develop/docs/cypress.md that should get you started.

@rashmitpankhania
Copy link
Contributor

Can you write a cypress test for this in https://github.com/matrix-org/matrix-react-sdk/blob/7a7e1fb43b49b994b1c901e9151125d0642f5a52/cypress/e2e/settings/general-room-settings-tab.spec.ts? You can assert that the width of the select element is less than that of .mx_Dialog.

Okay,I'll try

Hi @ABHIXIT2
You can add assertion like below rashmitpankhania@fe7f2f0

@ABHIXIT2
Copy link
Contributor Author

ABHIXIT2 commented Oct 5, 2023

Can you write a cypress test for this in https://github.com/matrix-org/matrix-react-sdk/blob/7a7e1fb43b49b994b1c901e9151125d0642f5a52/cypress/e2e/settings/general-room-settings-tab.spec.ts? You can assert that the width of the select element is less than that of .mx_Dialog.

I've added the test but I wasn't sure about the message and comments so I followed other commits .

@MidhunSureshR
Copy link
Contributor

Thanks but you need to first set the room-address to something really long before checking for overflow:

    it("long address should not cause dialog to overflow", () => {
        cy.openRoomSettings("General");
        // 1. Set the room-address to be a really long string
        const longString =
        "abcasdhjasjhdaj1jh1asdhasjdhajsdhjavhjksdnfjasdhfjh21jh3j12h3jashfcjbabbabasdbdasjh1j23hk1l2j3lamajshdjkltyiuwioeuqpirjdfmngsdnf8378234jskdfjkdfnbnsdfbasjbdjashdajshfgngnsdkfsdkkqwijeqiwjeiqhrkldfnaskldklasdn";
        cy.get("#roomAliases").within(() => {
            cy.get("input[label='Room address']").type(longString);
            cy.contains("Add").click();
        });

        // 2. wait for the new setting to apply ...
        cy.get("#canonicalAlias").should("have.value", `#${longString}:localhost`);

        // 3. Check if the dialog overflows
        cy.get(".mx_Dialog").invoke("outerWidth").then((dialogWidth) => {
            cy.get("#canonicalAlias").invoke("outerWidth").then((fieldWidth) => {
                // Assert that the width of the select element is less than that of .mx_Dialog div.
                expect(fieldWidth).to.be.lessThan(dialogWidth);
            });
        });
    });

@MidhunSureshR
Copy link
Contributor

Format cypress/e2e/settings/general-room-settings-tab.spec.ts with prettier and we're good to merge!

Copy link
Contributor

@MidhunSureshR MidhunSureshR left a comment

Choose a reason for hiding this comment

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

Thank you 🎉

@ABHIXIT2
Copy link
Contributor Author

ABHIXIT2 commented Oct 7, 2023

Thank you @MidhunSureshR @rashmitpankhania @andybalaam for the help.

@MidhunSureshR MidhunSureshR added this pull request to the merge queue Oct 9, 2023
Merged via the queue into matrix-org:develop with commit 54ca20d Oct 9, 2023
16 checks passed
@ABHIXIT2 ABHIXIT2 deleted the Abhixit2/issue25614 branch October 9, 2023 12:08
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Nov 4, 2023
Changes in [1.11.47](https://github.com/vector-im/element-web/releases/tag/v1.11.47) (2023-10-24)
=================================================================================================

## 🦖 Deprecations
 * Deprecate customisations in favour of Module API ([\#25736](element-hq/element-web#25736)). Fixes #25733.

## ✨ Features
 * element-hq/element-x-ios/issues/1824 - Convert the apple-app-site-association file to a newer format… ([\#26307](element-hq/element-web#26307)). Contributed by @stefanceriu.
 * Iterate `io.element.late_event` decoration ([\#11760](matrix-org/matrix-react-sdk#11760)). Fixes #26384.
 * Render timeline separator for late event groups ([\#11739](matrix-org/matrix-react-sdk#11739)).
 * OIDC: revoke tokens on logout ([\#11718](matrix-org/matrix-react-sdk#11718)). Fixes #25394. Contributed by @kerryarchibald.
 * Show `io.element.late_event` in MessageTimestamp when known ([\#11733](matrix-org/matrix-react-sdk#11733)).
 * Show all labs flags if developerMode enabled ([\#11746](matrix-org/matrix-react-sdk#11746)). Fixes #24571 and #8498.
 * Use Compound tooltips on MessageTimestamp to improve UX of date time discovery ([\#11732](matrix-org/matrix-react-sdk#11732)). Fixes #25913.
 * Consolidate 4s passphrase input fields and use stable IDs ([\#11743](matrix-org/matrix-react-sdk#11743)). Fixes #26228.
 * Disable upgraderoom command without developer mode enabled ([\#11744](matrix-org/matrix-react-sdk#11744)). Fixes #17620.
 * Avoid rendering app download buttons if disabled in config ([\#11741](matrix-org/matrix-react-sdk#11741)). Fixes #26309.
 * OIDC: refresh tokens ([\#11699](matrix-org/matrix-react-sdk#11699)). Fixes #25839. Contributed by @kerryarchibald.
 * OIDC: register ([\#11727](matrix-org/matrix-react-sdk#11727)). Fixes #25393. Contributed by @kerryarchibald.
 * Use stable get_login_token and remove unstable MSC3882 support ([\#11001](matrix-org/matrix-react-sdk#11001)). Contributed by @hughns.

## 🐛 Bug Fixes
 * Set max size for Element logo in search warning ([\#11779](matrix-org/matrix-react-sdk#11779)). Fixes #26408.
 * Avoid error when DMing oneself ([\#11754](matrix-org/matrix-react-sdk#11754)). Fixes #7242.
 * Fix: Message shield alignment is not right. ([\#11703](matrix-org/matrix-react-sdk#11703)). Fixes #26142. Contributed by @manancodes.
 * fix logging full event ([\#11755](matrix-org/matrix-react-sdk#11755)). Fixes #26376.
 * OIDC: use delegated auth account URL from `OidcClientStore` ([\#11723](matrix-org/matrix-react-sdk#11723)). Fixes #26305. Contributed by @kerryarchibald.
 * Fix: Members list shield alignment is not right. ([\#11700](matrix-org/matrix-react-sdk#11700)). Fixes #26261. Contributed by @manancodes.
 * Fix: <detail> HTML elements clickable area too wide. ([\#11666](matrix-org/matrix-react-sdk#11666)). Fixes #25454. Contributed by @manancodes.
 * Fix untranslated headings in the devtools dialog ([\#11734](matrix-org/matrix-react-sdk#11734)).
 * Fixes invite dialog alignment and pill color contrast ([\#11722](matrix-org/matrix-react-sdk#11722)). Contributed by @gabrc52.
 * Prevent select element in General settings overflowing in a room with very long room-id ([\#11597](matrix-org/matrix-react-sdk#11597)). Contributed by @ABHIXIT2.
 * Fix: Clicking on members pile does nothing. ([\#11657](matrix-org/matrix-react-sdk#11657)). Fixes #26164. Contributed by @manancodes.
 * Fix: Wierd shadow below room avatar in dark mode. ([\#11678](matrix-org/matrix-react-sdk#11678)). Fixes #26153. Contributed by @manancodes.
 * Fix start_sso / start_cas URLs failing to redirect to a authentication prompt ([\#11681](matrix-org/matrix-react-sdk#11681)). Contributed by @Half-Shot.

Changes in [1.11.46](https://github.com/vector-im/element-web/releases/tag/v1.11.46) (2023-10-10)
=================================================================================================

## ✨ Features
 * Use .well-known to discover a default rendezvous server for use with Sign in with QR ([\#11655](matrix-org/matrix-react-sdk#11655)). Contributed by @hughns.
 * Message layout will update according to the selected style  ([\#10170](matrix-org/matrix-react-sdk#10170)). Fixes #21782. Contributed by @manancodes.
 * Implement MSC4039: Add an MSC for a new Widget API action to upload files into the media repository ([\#11311](matrix-org/matrix-react-sdk#11311)). Contributed by @dhenneke.
 * Render space pills with square corners to match new avatar ([\#11632](matrix-org/matrix-react-sdk#11632)). Fixes #26056.
 * Linkify room topic ([\#11631](matrix-org/matrix-react-sdk#11631)). Fixes #26185.
 * Show knock rooms in the list ([\#11573](matrix-org/matrix-react-sdk#11573)). Contributed by @maheichyk.

## 🐛 Bug Fixes
 * Bump matrix-web-i18n dependency to 3.1.3 ([\#26287](element-hq/element-web#26287))
 * Fix: Avatar shrinks with long names ([\#11698](matrix-org/matrix-react-sdk#11698)). Fixes #26252. Contributed by @manancodes.
 * Update custom translations to support nested fields in structured JSON ([\#11685](matrix-org/matrix-react-sdk#11685)).
 * Fix: Edited message remove button is hard to reach. ([\#11674](matrix-org/matrix-react-sdk#11674)). Fixes #24917. Contributed by @manancodes.
 * Fix: Theme selector radio button not aligned in center with the text ([\#11676](matrix-org/matrix-react-sdk#11676)). Fixes #25460. Contributed by @manancodes.
 * Fix: Unread notification dot aligned ([\#11658](matrix-org/matrix-react-sdk#11658)). Fixes #25285. Contributed by @manancodes.
 * Fix: sync intentional mentions push rules with legacy rules ([\#11667](matrix-org/matrix-react-sdk#11667)). Fixes #26227. Contributed by @kerryarchibald.
 * Revert "Fix regression around FacePile with overflow (#11527)" ([\#11634](matrix-org/matrix-react-sdk#11634)). Fixes #26209.
 * Fix: Alignment Fixed ([\#11648](matrix-org/matrix-react-sdk#11648)). Fixes #26169. Contributed by @manancodes.
 * Fix: onFinished added which closes the menu ([\#11647](matrix-org/matrix-react-sdk#11647)). Fixes #25556. Contributed by @manancodes.
 * Don't start key backups when opening settings ([\#11640](matrix-org/matrix-react-sdk#11640)).
 * Fix add to space avatar text centering ([\#11643](matrix-org/matrix-react-sdk#11643)). Fixes #26154.
 * fix avatar styling in lightbox ([\#11641](matrix-org/matrix-react-sdk#11641)). Fixes #26196.
@BhargavaCreates
Copy link

I am new to OSC, and this PR showed me how to work collaboratively and seek help. Thanks @ABHIXIT2 @MidhunSureshR @andybalaam for setting such good example.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants