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

Background in ranking items dialog in rank widget is white #6047

Closed
dbemke opened this issue Mar 27, 2024 · 16 comments · Fixed by #6178
Closed

Background in ranking items dialog in rank widget is white #6047

dbemke opened this issue Mar 27, 2024 · 16 comments · Fixed by #6178
Assignees
Milestone

Comments

@dbemke
Copy link

dbemke commented Mar 27, 2024

ODK Collect version

the master version 38a4b0e

Android version

10, 14

Device used

Redmi t,Pixel 7a

Problem description

In rank widget in the dialog with choices all colors in the background changed to white.
The issue doesn’t occur in the store version 2024.1.3
rank

Steps to reproduce the problem

  1. Set light mode in Collect.
  2. Go to All widgets form to rank widget.
  3. Tap "Rank items".

Expected behavior

store

@grzesiek2010
Copy link
Member

The color of items didn't change but the color of the dialog did (#6025). Now it's more gray and that's why the difference is not visible enough.
@alyblenkin what do you think? What color would you use other (for the items that you can rank)?

@alyblenkin
Copy link
Collaborator

The color of items didn't change but the color of the dialog did (#6025). Now it's more gray and that's why the difference is not visible enough. @alyblenkin what do you think? What color would you use other (for the items that you can rank)?

Good spot! Could we make it the same gray as the text fields, which I believe is our surface container highest.

@grzesiek2010
Copy link
Member

Could we make it the same gray as the text fields, which I believe is our surface container highest.

This won't be enough. The difference between colorSurfaceContainerHighest and colorSurfaceContainerHigh (the color we used to display dialogs) is not visible enough.
Screenshot_1711638743

@alyblenkin
Copy link
Collaborator

Got it. Do we need to adjust the highest surface container to make the difference between them darker?

I believe we made the highest is #F2F2F2, so if we try #E0E0E0 or #EBEBEB the difference should be darker.

@grzesiek2010
Copy link
Member

#EBEBEB #E0E0E0
Screenshot_1711643606 Screenshot_1711643680

I think the darker one looks better. The first one might not be visible in daylight. Any thoughts @alyblenkin?
colorSurfaceContainerHighest is the one we use to display the banner in the form end view and the no-errors pill (see related issue #6046). @alyblenkin what colors should we use in those cases instead?

@grzesiek2010
Copy link
Member

@alyblenkin I have the same problem with #5845. The bottom sheet has that Learn more about MBTiles info section:
image
and if I try to use the gray color we use in the form end view:
image
It's not visible because the bottom sheet dialog has a darker background by default like normal dialogs.

It would be good to decide what color should be used in such cases.

@alyblenkin
Copy link
Collaborator

It's not visible because the bottom sheet dialog has a darker background by default like normal dialogs.

That's too bad - the new colour mapping isn't ideal in all cases! I'm not sure we want to introduce another colour. Another alternative I can think of is creating a divider between the info section and the actions.

@grzesiek2010
Copy link
Member

Another alternative I can think of is creating a divider between the info section and the actions.

I can do that for that new bottom sheet but this does not solve the problem with the ranking question type.

@alyblenkin
Copy link
Collaborator

@grzesiek2010 @seadowg, I'm wondering if we should adjust the surface colours again because they're making things pretty tricky.

We need to figure out a way to create more colour options because we need a higher contrast for the ranking question type, and #E0E0E0 works well. However, we want the message on the end screen to be much lighter because the text is harder to read with a darker background. #EBEBEB is probably the darkest we would want for the messages because we had to switch it from blue to grey. Another idea is to remove the background colour for the messages so they are consistent throughout (end screen and bottom sheet). What do you think? Happy to chat about it too if that's easier!

@seadowg
Copy link
Member

seadowg commented Apr 25, 2024

@alyblenkin We're currently in a situation where our "surface container low", "surface container" and "surface container high" are all the same so that's probably leading to some of these problems with contrast. It might be worth taking a step back and redefining the whole color set.

When we converted to Material 3, the Android components library wasn't using the tonal surface colors yet so we weren't able to use the Theme Builder properly where as we can now. Maybe it would be good to try defining a color palette there and export for the "Android Views (XML)" for us and we can define all the colors in one go?

@alyblenkin
Copy link
Collaborator

Capturing the summary from our group chat on slack here:

@seadowg we will find time to pair on plugging in the colours we already use into the tonal palette and hopefully it generates something that works well.

@grzesiek2010 I don't think it's a huge deal for use to use the lighter grey for now for the ranking question until we sort out the colours. And as Callum said, the worse case is the beta goes out with the ranking problem.

@alyblenkin
Copy link
Collaborator

@grzesiek2010 - I wanted to give a quick update on this one.

@seadowg and I briefly tried the theme builder and added our colours, and it seems better than what they had before! It's not perfect and will need some tweaking, but they also now have a figma plugin that I can play around with to propose different options. I'll aim to do that as part of this release so that we can unblock this issue and avoid running into these same issues.

@seadowg
Copy link
Member

seadowg commented Jun 7, 2024

Let's go with using colorSurfaceContainerHighest for now (like in #6047 (comment)). We're looking to increase the contrast between the tonal colors, so this should get better when we do that next release.

@alyblenkin
Copy link
Collaborator

Let's go with using colorSurfaceContainerHighest for now (like in #6047 (comment)). We're looking to increase the contrast between the tonal colors, so this should get better when we do that next release.

Did you happen to see this #6171, @seadowg? I think the drag icon will significantly improve this interaction.

@seadowg
Copy link
Member

seadowg commented Jun 10, 2024

Did you happen to see this #6171, @seadowg? I think the drag icon will significantly improve this interaction.

Yeah it's definitely nicer! I think we want a quick fix option we can apply before this next release though, so I'm still in favour of making the color change first.

@alyblenkin
Copy link
Collaborator

alyblenkin commented Jun 12, 2024

Yeah it's definitely nicer! I think we want a quick fix option we can apply before this next release though, so I'm still in favour of making the color change first.

Makes sense!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: done
Development

Successfully merging a pull request may close this issue.

4 participants