Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Invite dialog margins are visually inconsistent #20631

Closed
robintown opened this issue Jan 19, 2022 · 13 comments · Fixed by matrix-org/matrix-react-sdk#8063 or matrix-org/matrix-react-sdk#8076
Closed

Invite dialog margins are visually inconsistent #20631

robintown opened this issue Jan 19, 2022 · 13 comments · Fixed by matrix-org/matrix-react-sdk#8063 or matrix-org/matrix-react-sdk#8076
Labels
A-Appearance A-Invite O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience S-Tolerable Low/no impact on users T-Defect X-Needs-Design

Comments

@robintown
Copy link
Member

Steps to reproduce

Open an invite dialog

Outcome

Screenshot 2022-01-18 at 23-09-02 Element general

What did you expect?

The dialog should have a uniform margin on all 4 sides, matching the visual style of all other dialogs in the application.

What happened instead?

The dialog seems as if all 4 sides have a different margin, which looks unusual. Additionally, there seem to be two different margin widths in use on the right side. (The 'invite' button and 'X hours ago' labels are indented further than other elements.)

Operating system

NixOS unstable

Browser information

Firefox 96.0

URL for webapp

develop.element.io

Application version

Element version: 840867b-react-a00d359422a6-js-652b3a9208f0 Olm version: 3.2.8

Homeserver

townsendandsmith.ml

Will you send logs?

No
@robintown robintown added T-Defect S-Tolerable Low/no impact on users X-Needs-Design A-Invite A-Appearance O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience labels Jan 19, 2022
@luixxiul luixxiul mentioned this issue Mar 16, 2022
Merged
@robintown
Copy link
Member Author

Reopening as e.g. the margins on the top and left are still different

@robintown robintown reopened this Mar 16, 2022
@luixxiul
Copy link

Which margin are you talking about?

after

The top (above Invite ...) and the left (left of Recently ... and Suggetions)?

@robintown
Copy link
Member Author

Yes, that's correct

@robintown
Copy link
Member Author

See matrix-org/matrix-react-sdk#6111 (comment) for the layout that was being discussed by the designers

@andybalaam
Copy link
Contributor

Hmm, seems like I should have passed the PR to the designers after all, sorry about that.

@luixxiul
Copy link

@niquewoodhouse how much padding on the top above the heading is preferred? I checked matrix-org/matrix-react-sdk#6111 (comment) but I cannot really find the instruction about that.

@luixxiul
Copy link

Hmm, seems like I should have passed the PR to the designers after all, sorry about that.

That's what I missed, you should not be sorry...

@luixxiul
Copy link

By setting the same padding on the top and on the left:

margin

Is this fine?

@robintown
Copy link
Member Author

It would probably be better to simply remove the extra left and right padding on the dialog's contents, so that it just uses the standard 24px padding on all sides.

@robintown
Copy link
Member Author

(as in this image from that PR I linked to:)

padding

@luixxiul
Copy link

I have not been so sure about it due to // the design wants some padding on the left on https://github.com/luixxiul/matrix-react-sdk/blob/7cea71085e4ba4bf5eed8427cece76b607c3a753/res/css/views/dialogs/_InviteDialog.scss#L276. Is it no longer valid?

@robintown
Copy link
Member Author

Right, that's why this will need input from the design team. If they approve of the changes, it won't be valid anymore.

@niquewoodhouse
Copy link

(as in this image from that PR I linked to:)

padding

To confirm, this image is as desired by design, thanks. I believe the // the design wants some padding on the left is an out of date note left in the file and all that bespoke padding should be removed. I'm happy to review if that helps.

@luixxiul luixxiul mentioned this issue Mar 17, 2022
Merged
turt2live pushed a commit to matrix-org/matrix-react-sdk that referenced this issue Mar 18, 2022
@luixxiul
Remove padding of InviteDialog & fix visual regression (#8076)
d7a3f39 
* Remove padding of InviteDialog

Closes element-hq/element-web#20631

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>

* Fix visual regression of InviteDialog

Signed-off-by: Suguru Hirahara <luixxiul@users.noreply.github.com>
su-ex added a commit to SchildiChat/element-web that referenced this issue Apr 1, 2022
@su-ex
Merge tag 'v1.10.8' into sc
2c28fa8 
* Live location sharing: live share warning in room ([\element-hq#8100](matrix-org/matrix-react-sdk#8100)).
* Add simple live share warning ([\element-hq#8066](matrix-org/matrix-react-sdk#8066)).
* extract reusable styled live beacon icon ([\element-hq#8103](matrix-org/matrix-react-sdk#8103)).
* Don't restore MemberInfo from RightPanel history when viewing a room ([\element-hq#8090](matrix-org/matrix-react-sdk#8090)). Fixes element-hq#21487.
* Allow sending files as replies as per MSC3676 ([\element-hq#8020](matrix-org/matrix-react-sdk#8020)). Fixes element-hq#7156.
* kill beacons on expiry ([\element-hq#8075](matrix-org/matrix-react-sdk#8075)).
* enable geolocation behaviour in location picker for live share type ([\element-hq#8068](matrix-org/matrix-react-sdk#8068)).
* Improve formatting features in the editor ([\#7104](matrix-org/matrix-react-sdk#7104)). Fixes element-hq#19501. Contributed by @alexanderstephan.
* Support MSC3026 busy presence ([\element-hq#8043](matrix-org/matrix-react-sdk#8043)).
* Show displayname in non-narrow thread summeries ([\element-hq#8036](matrix-org/matrix-react-sdk#8036)). Fixes element-hq#19646.
* Tweak search dialog based on new designs ([\element-hq#7980](matrix-org/matrix-react-sdk#7980)). Fixes element-hq#21285 and element-hq#21289.
* fallback to event text in location body when map unavailable ([\element-hq#7982](matrix-org/matrix-react-sdk#7982)). Fixes element-hq#20655.
* Send pin drop location share events ([\#7967](matrix-org/matrix-react-sdk#7967)).
* fix quicktime video thumbnailing ([\element-hq#8108](matrix-org/matrix-react-sdk#8108)). Fixes element-hq#21505.
* Fix scroll behaviour in space panel ([\element-hq#8111](matrix-org/matrix-react-sdk#8111)). Fixes element-hq#21467.
* Fix emoting with emoji or pills ([\element-hq#8105](matrix-org/matrix-react-sdk#8105)). Fixes element-hq#21497.
* Remove padding of InviteDialog & fix visual regression ([\element-hq#8076](matrix-org/matrix-react-sdk#8076)). Fixes element-hq#20631. Contributed by @luixxiul.
* Fixes mx_MLocationBody_markerBorder ([\element-hq#8069](matrix-org/matrix-react-sdk#8069)). Fixes element-hq#21444. Contributed by @luixxiul.
* Make margin and padding of mx_InviteDialog_other consistent ([\#8063](matrix-org/matrix-react-sdk#8063)). Fixes element-hq#20631. Contributed by @luixxiul.
* Fix freeze/crash when 1:1 calling ([\element-hq#8057](matrix-org/matrix-react-sdk#8057)). Fixes element-hq#21181.
* Don't assume that widget IDs are unique ([\#8052](matrix-org/matrix-react-sdk#8052)). Fixes element-hq#21399.
* Fix the header of Space landing page ([\element-hq#8048](matrix-org/matrix-react-sdk#8048)). Fixes element-hq#21402. Contributed by @luixxiul.
* Fix buttons alignment of Space list header ([\element-hq#8047](matrix-org/matrix-react-sdk#8047)). Fixes element-hq#21401. Contributed by @luixxiul.
* Fix null-guarding regression around reply_to_event dispatch ([\element-hq#8039](matrix-org/matrix-react-sdk#8039)).
* Fix clicking on copy link to thread wrongly opening thread ([\element-hq#8038](matrix-org/matrix-react-sdk#8038)). Fixes element-hq#20653.
* Fix regression around replying to search results ([\element-hq#8035](matrix-org/matrix-react-sdk#8035)). Fixes element-hq#21389.
* Share shared history keys in the background ([\element-hq#8031](matrix-org/matrix-react-sdk#8031)). Fixes element-hq#21192.
* Paginate responses to pinned polls ([\element-hq#8025](matrix-org/matrix-react-sdk#8025)). Fixes element-hq#21382.
* Fix incorrect usage of unstable variant of `is_falling_back` ([\element-hq#8016](matrix-org/matrix-react-sdk#8016)).
* Fix issues with ThreadSummary in msc-enabled mode ([\element-hq#8018](matrix-org/matrix-react-sdk#8018)). Fixes matrix-org/element-web-rageshakes#11401 and matrix-org/element-web-rageshakes#11400.
* Fix alignment of polls within threads ([\element-hq#8017](matrix-org/matrix-react-sdk#8017)). Fixes element-hq#21235.
* Fix issues with thread summaries being wrong or stale ([\element-hq#8015](matrix-org/matrix-react-sdk#8015)). Fixes element-hq#21363 and element-hq#21204.
* Fix button border color of LeaveSpaceDialog ([\element-hq#8010](matrix-org/matrix-react-sdk#8010)). Fixes element-hq#21365. Contributed by @luixxiul.
* Fix room list scroll jumps ([\element-hq#7991](matrix-org/matrix-react-sdk#7991)). Fixes element-hq#19322.
* Fix a variety of issues with HTML → Markdown conversion ([\element-hq#8004](matrix-org/matrix-react-sdk#8004)). Fixes element-hq#10648, element-hq#20718, element-hq#10722, element-hq#10389, element-hq#17610 element-hq#9984 and element-hq#20140.
* Wrap EventTile rather than its children in an error boundary ([\element-hq#7945](matrix-org/matrix-react-sdk#7945)).
* Normalized shortcut formatting for quote expansion control ([\element-hq#7995](matrix-org/matrix-react-sdk#7995)). Fixes element-hq#19685. Contributed by @Sinharitik589.
* Fix buttons and text layout on Security Key dialog ([\element-hq#7996](matrix-org/matrix-react-sdk#7996)). Fixes element-hq#21330. Contributed by @luixxiul.
* Fix formatting not being applied after links ([\element-hq#7990](matrix-org/matrix-react-sdk#7990)). Fixes element-hq#20091.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-Appearance A-Invite O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience S-Tolerable Low/no impact on users T-Defect X-Needs-Design
Projects
None yet
Milestone
No milestone
4 participants
@andybalaam @niquewoodhouse @luixxiul @robintown