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.

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

Transparent avatars have solid background in the timeline #26159

Closed
Johennes opened this issue Sep 13, 2023 · 13 comments
Closed

Transparent avatars have solid background in the timeline #26159

Johennes opened this issue Sep 13, 2023 · 13 comments
Assignees
Labels
A-Avatar A-Timeline O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect X-Regression

Comments

@Johennes
Copy link
Contributor

Starting with 1.11.42 transparent avatars stopped working. They do render like before in the top left and settings.

Screenshot 2023-09-13 at 08 59 54

Screenshot 2023-09-13 at 08 59 38

But in the timeline they have a grey background instead of transparency.

Screenshot 2023-09-13 at 08 59 45

@Johennes
Copy link
Contributor Author

Discussed with @nadonomy and @callumu this morning and the desired resolution is:

  • Remove the grey background
  • Add a grey300 border around all avatars as shown here

Screenshot 2023-09-13 at 13 57 33

@nadonomy
Copy link
Contributor

nadonomy commented Sep 13, 2023

  • Add a grey300 border around all avatars as shown here

One note on this - we're gonna quickly sense check the exact values early next week. The comp here is using alpha/300 and I'm not sure if it's desirably or undesirably feint. Also will check how to handle the coloured avatars.

@AdamRGrey
Copy link

why add the border? why not... nothing, like before, which was better?

@olmari
Copy link

olmari commented Sep 13, 2023

This is someone elses Avatar, but this issue is also way more pronounced in Dark theme:

image

While opened up the actual image:

image

@nadonomy
Copy link
Contributor

why add the border? why not... nothing, like before, which was better?

The timeline is more legible when there are clear content keylines for the eye to follow up and down, consistently. Transparent avatars, or avatars which perfectly match the background (e.g. white on white) break this alignment.

So, we'll add some feint visual structure in the form of a subtle stroke to help. This way transparent avatars can maintain their content, unlike the fills above.

@AdamRGrey
Copy link

so "we don't like how transparent avatars look, if you choose a transparent avatar we'll override you", gotcha

@robintown robintown added S-Minor Impairs non-critical functionality or suitable workarounds exist A-Timeline A-Avatar O-Occasional Affects or can be seen by some users regularly or most users rarely labels Sep 13, 2023
@janogarcia
Copy link

On the light gray background for transparent avatars. It's a bug in the component implementation. It seems it's currently inheriting the default user agent style for the button element background. Just for clarity, there was no design spec stating that transparent avatars should get a solid background.

On adding a translucent border for avatars (to prevent avatars from blending with the background and/or normalize their shape). This very same page (I mean, GitHub) is using a similar treatment for avatars. We don't strictly need it, but we'll do some explorations (and may discard it). That said, that should be a separate discussion from this issue as it's not specific to transparent avatars.

@janogarcia
Copy link

Resetting the button background to transparent would basically fix this issue.

@rda0
Copy link
Contributor

rda0 commented Sep 15, 2023

Same problem in 1.11.43 on Desktop in Dark mode. All our bot avatars with transparent background now look really bad due to the light-grey background added:

image

@germain-gg
Copy link
Contributor

I'm not sure adding a border really solves the issue here, as we have to consider what has been raised as part of #26163.

IMO we should set the avatar background to be --cpd-color-bg-canvas-default

@nadonomy / @janogarcia ?

@janogarcia
Copy link

@germain-gg It's already scheduled for discussion (tomorrow, if there are not last minute changes to the agenda) and documented in Figma.

@germain-gg
Copy link
Contributor

After discussing this with Nad and Jano out of bands we are going to implement the suggestion of settings the canvas default color as this will have a broadest impact and ensure better visual consistency throughout the app.

@germain-gg
Copy link
Contributor

Fixed as compound as been upgraded

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Avatar A-Timeline O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect X-Regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants