Skip to content

fix: ui overflow issues in Create Group modal [WPB-22420]#20893

Open
theham3d wants to merge 2 commits intowireapp:devfrom
theham3d:fix/overflow-create-group-issue
Open

fix: ui overflow issues in Create Group modal [WPB-22420]#20893
theham3d wants to merge 2 commits intowireapp:devfrom
theham3d:fix/overflow-create-group-issue

Conversation

@theham3d
Copy link
Copy Markdown

Summary

Hi team 👋

I recently came across an open position at Wire, and before applying, I wanted to get a better feel for the product and the codebase. I usually like to explore the app a bit and check out the website, so I did the same here.

While playing around with the app, I noticed two small UI issues in Create Group modal. The first one is in the header, it slightly overflows the modal body and causes a small visual glitch in the corner. The second one happens when we're showing list of users: the modal becomes scrollable, but the current styling creates some inconsistencies, with overlapping overflow areas and slightly awkward scrolling.

What I changed:

  • Removed the wrapperCSS property from the modal component (which sets overflow: unset)
  • Adjusted the modal body styling to improve scrolling behavior when the user list grows

From what I could tell and checked the history, wrapperCSS was originally introduced for FadingScrollbar, but in this case it doesn’t seem to add value, and removing it didn’t introduce any issues.

Result:

  • More consistent and predictable scrolling behavior
  • Cleaner UI in the modal when the list grows
  • Fixed header overflow glitch

I’ve attached before/after screenshots for reference in below.


Security Checklist (required)

  • External inputs are validated & sanitized on client and/or server where applicable.
  • API responses are validated; unexpected shapes are handled safely (fallbacks or errors).
  • No unsafe HTML is rendered; if unavoidable, sanitization is applied and documented where it happens.
  • Injection risks (XSS/SQL/command) are prevented via safe APIs and/or escaping.

Accessibility (required)

Standards Acknowledgement (required)


Screenshots or demo (if the user interface changed)

Before:

scroll-issue dark-o light-o

After:

fix

Notes for reviewers

  • Trade-offs:
  • Follow-ups (linked issues):
  • Linked PRs (e.g. web-packages):

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 30, 2026

CLA assistant check
All committers have signed the CLA.

@theham3d theham3d changed the title Fix: ui overflow issues in Create Group modal fix: ui overflow issues in Create Group modal Mar 30, 2026
overflow-y: auto;
}
}
} No newline at end of file
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@theham3d could you fix the EOL issue here please?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@sonarqubecloud
Copy link
Copy Markdown

@theham3d theham3d requested a review from thisisamir98 March 30, 2026 15:12
line-height: var(--line-height-lg);
}

.modal__body {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

removing wrapperCSS={{overflow: 'unset', overflowY: 'unset'}} is necessary because this was overriding overflow: hidden of the wrapper.

adding modal_body{ overflow: hidden} could be dropped because scrolling is handled inside .group-creation &__list which already has

height: 374px;
overflow-y: auto;

could you explain why we need .modal__body {overflow-y: hidden;} too?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks @arjita-mitra for the review 👍
Yeah, you’re right. From what I can see in the history, this was mainly introduced because of the FadingScrollbar component. It looks like that changed over time, and when you fixed the issue, this override was left behind.

As for your question, the main reason is that you’re already handling the height limit and overflow in .group-creation__list, like you mentioned. With the current setup, the active style on .modal__body ends up causing a double scroll. In reality, there’s only one scrollable area (the list), but the UI shows two scrollbars, which is just a visual glitch (I've atached a screenshot above). This override fixes that issue.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I couldn't reproduce the double scrollbar though. Please check the pipeline issues.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey @theham3d
I am also unable to reproduce the double scrollbar.
Can you provide some steps, browser details, resolution?
The other part of this PR is good to go which is artifacts at the top corners of modal.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@arjita-mitra @zskhan Thanks for the review.
I’ve recorded a video on the live production app to show how this can be reproduced. Since I didn’t have enough contacts to make the list scrollable, I duplicated them manually to demonstrate it.

Just a note about the video, the file was over 10MB, so I uploaded it to another service. But will expire by the end of today due to platform limits.

If you want to reproduce it yourself, you can try it with a scrollable contact list in the group creation flow, you’ll see the double scroll issue. If not, you can also add contacts manually like I did.

After applying the fix, the double scroll should be gone. I’m using Chrome version 146.0.7680.165.

Here is the video link: https://vimeo.com/reviews/bd7d8d76-26ac-4063-a6e7-35baa9eb8eaf/videos/1181857734

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

For the failed pipeline, aside from the Jira ticket number check, I think the other issue might be because the PR is coming from a fork. It probably needs someone with elevated permissions to run it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Quick update as of April 13: I just checked the link again. Based on the platform message I thought the preview would expire by the end of Friday, but it’s still working on my side in incognito tab.

Let me know if you have any trouble viewing the video.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hello.
I saw the video and now I am able to reproduce it.
About the PR CI issues I will also take care of it.
Thanks.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@zskhan Thanks 💯

@thisisamir98 thisisamir98 changed the title fix: ui overflow issues in Create Group modal fix: ui overflow issues in Create Group modal [WPB-22420] Mar 31, 2026
@theham3d theham3d requested a review from arjita-mitra March 31, 2026 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants