Skip to content

Fix transparent verification checkmark in dark mode #30235

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

Merged
merged 2 commits into from
Jul 7, 2025
Merged

Fix transparent verification checkmark in dark mode #30235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
20fa298
Fix transparent verification checkmark in dark mode
Banbuii Jul 1, 2025
7a9376b
Add white background to E2E Warning Icon
Banbuii Jul 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions res/css/views/rooms/_E2EIcon.pcss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,10 @@ Please see LICENSE files in the repository root for full details.
mask-image: url("$(res)/img/e2e/verified.svg");
background-color: $e2e-verified-color;
}

// When using the "normal" icon as a background for verified or warning icon,
// it should be slightly smaller than the foreground icon
.mx_E2EIcon_verified, .mx_E2EIcon_warning .mx_E2EIcon_normal::after {
mask-size: 90%;
background-color: white;
}
12 changes: 11 additions & 1 deletion src/components/views/rooms/E2EIcon.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,17 @@ const E2EIcon: React.FC<Props> = ({
if (onClick) {
content = <AccessibleButton onClick={onClick} className={classes} style={style} />;
} else {
content = <div className={classes} style={style} />;
// Verified and warning icon have a transparent cutout, so add a white background.
// The normal icon already has the correct shape and size, so reuse that.
if (status === E2EStatus.Verified || status === E2EStatus.Warning) {
content = (
<div className={classes} style={style}>
<div className="mx_E2EIcon_normal" />
</div>
);
} else {
content = <div className={classes} style={style} />;
}
Copy link
Member

Choose a reason for hiding this comment

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

Not sure I understand this change: aren't the classes supposed to correspond to the state of the icon, so normal and verified are separate states? Why is this adding normal when it's verified? Also, why the nested div?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just used the "normal" Icon, to put it behind the verified icon, to use it as a white background. I didn't really know how else to do it, except to use a white rectangle or something. But in any case I needed some kind of additional div to use a a background, because otherwise, the whole box around the verified shield would be white, and that looked bad (in my opinion).
And the "normal" icon already had the right form factor.

But I can change the Icon to a new div with a smaller rectangle. I just thought it would be better to reuse existing icons, but as said, I can easily change that.

Copy link
Member

Choose a reason for hiding this comment

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

Aha! I hadn't realised that's what this was doing. I think this could definitely do with a comment! I think it will also need the same with the warning state of the icon, as that has a '!' cutout in the shield.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then I will add it to that as well

}

if (!e2eTitle || hideTooltip) {
Expand Down
6 changes: 5 additions & 1 deletion test/unit-tests/components/structures/__snapshots__/RoomView-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1809,7 +1809,11 @@ exports[`RoomView should not display the timeline when the room encryption is lo
<div
aria-labelledby="«r3e»"
class="mx_E2EIcon mx_E2EIcon_verified mx_MessageComposer_e2eIcon"
/>
>
<div
class="mx_E2EIcon_normal"
/>
</div>
Copy link
Member

Choose a reason for hiding this comment

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

Again, why another nested div?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The nested div is for the background, to not have the white background being bigger than the verified icon.

</span>
</div>
<div
Expand Down
12 changes: 10 additions & 2 deletions test/unit-tests/components/views/dialogs/__snapshots__/UntrustedDeviceDialog-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ exports[`<UntrustedDeviceDialog /> should display the dialog for the device of a
<div
class="mx_E2EIcon mx_E2EIcon_warning"
style="width: 24px; height: 24px;"
/>
>
<div
class="mx_E2EIcon_normal"
/>
</div>
Not Trusted
</h1>
</div>
Expand Down Expand Up @@ -97,7 +101,11 @@ exports[`<UntrustedDeviceDialog /> should display the dialog for the device of t
<div
class="mx_E2EIcon mx_E2EIcon_warning"
style="width: 24px; height: 24px;"
/>
>
<div
class="mx_E2EIcon_normal"
/>
</div>
Not Trusted
</h1>
</div>
Expand Down
Loading