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

Alternate background colour of chat lines to better visually distinguish wrapped lines #29137

Merged

Conversation

normalid-awa
Copy link
Contributor

@normalid-awa normalid-awa commented Jul 27, 2024

Resolved: #29129 (first bullet point)

Master image
PR image

For better accessibility, altering the chat message background color with the message id, unit test were added

@normalid-awa normalid-awa changed the title Feature/visual/chatline background altering Message background altering Jul 27, 2024
@normalid-awa normalid-awa changed the title Message background altering Chatline background color pattern altering Jul 27, 2024
@peppy
Copy link
Member

peppy commented Jul 29, 2024

Doesn't look great. Would need some design input if we're going to do this. Also the corners of the highlight should probably be rounded, and the rows with no change to colour should probably not create needless drawables.

@normalid-awa
Copy link
Contributor Author

normalid-awa commented Jul 29, 2024

Doesn't look great. Would need some design input if we're going to do this. Also the corners of the highlight should probably be rounded, and the rows with no change to colour should probably not create needless drawables.

would go something like this?
image
image

added a little smoothness to the mask and made the box round

@bdach
Copy link
Collaborator

bdach commented Jul 29, 2024

paging @arflyte

@peppy
Copy link
Member

peppy commented Jul 29, 2024

With less rounding sure.

Apply the other changes I proposed and I'll fix up the visuals.

(I don't think this needs flyte input)

@normalid-awa
Copy link
Contributor Author

With less rounding sure.

Apply the other changes I proposed and I'll fix up the visuals.

(I don't think this needs flyte input)

The changes were done, the rest is the visual

@peppy peppy self-requested a review July 29, 2024 10:55
@pull-request-size pull-request-size bot added size/L and removed size/M labels Jul 29, 2024
@peppy
Copy link
Member

peppy commented Jul 29, 2024

Made some visual and behavioural changes.

peppy
peppy previously approved these changes Jul 29, 2024
@normalid-awa
Copy link
Contributor Author

normalid-awa commented Jul 29, 2024

omg, I didn't notice altering and alternating are two meanings.
Also, may I rename the unit test TestBackgroundAltering to TestChatlineBackgroundAlternating?It seems to make more sense and descriptive

@peppy peppy self-requested a review July 29, 2024 13:19
@peppy
Copy link
Member

peppy commented Jul 29, 2024

omg, I didn't notice altering and alternating are two meanings. Also, may I rename the unit test TestBackgroundAltering to TestChatlineBackgroundAlternating?It seems to make more sense and descriptive

Good catch, I've fixed that.

peppy
peppy previously approved these changes Jul 29, 2024
@bdach bdach self-requested a review July 29, 2024 14:19
@bdach
Copy link
Collaborator

bdach commented Jul 29, 2024

This looks so weak on dark backgrounds it may as well be imperceptible.

On daily challenge screen:

1722262970

In chat overlay:

1722262952

Unless there is a follow-up change to change daily challenge screen chat to have a lighter background, I don't think this fully achieves what it claims it does.

@peppy
Copy link
Member

peppy commented Jul 29, 2024

Might depend on screen gamma, since it doesn't look too bad here.

To improve the situation, we'll probably want to turn off additive blending and choose a more appropriate colour (white doesn't work well at higher opacities). I'll have a fiddle.

@peppy peppy self-requested a review July 30, 2024 05:58
@peppy
Copy link
Member

peppy commented Jul 30, 2024

I think this is feeling okay now, although there's some awkward overlap (admittedly was already an issue):

osu! 2024-07-30 at 06 58 03

I'll see what I can do about that while I'm here.

@peppy
Copy link
Member

peppy commented Jul 30, 2024

osu! 2024-07-30 at 07 17 26

osu! 2024-07-30 at 07 17 42

osu! 2024-07-30 at 07 19 03

@peppy peppy changed the title Chatline background color pattern altering Alternate background colour of chat lines to better visually distinguish Jul 30, 2024
@peppy peppy changed the title Alternate background colour of chat lines to better visually distinguish Alternate background colour of chat lines to better visually distinguish wrapped lines Jul 30, 2024
@peppy peppy force-pushed the feature/visual/chatline-background-altering branch 2 times, most recently from ae52ea1 to efb2e2e Compare July 30, 2024 07:15
@peppy peppy force-pushed the feature/visual/chatline-background-altering branch from efb2e2e to fc78dc9 Compare July 30, 2024 07:45
@peppy peppy merged commit 71acb7e into ppy:master Jul 30, 2024
11 of 13 checks passed
CloneWith added a commit to CloneWith/osu that referenced this pull request Aug 1, 2024
CloneWith added a commit to CloneWith/osu that referenced this pull request Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants