[Material You] "Select a folder" dialog - directory info row uses wrong color

min7-i commented 5 months ago

Checklist

[X] I can reproduce the bug with the latest version given here.
[X] I made sure that there are no existing issues - open or closed - to which I could contribute my information.
[X] I made sure that there are no existing discussions - open or closed - to which I could contribute my information.
[X] I have read the FAQs inside the app (Menu -> About -> FAQs) and my problem isn't listed.
[X] I have taken the time to fill in all the required details. I understand that the bug report will be dismissed otherwise.
[X] This issue contains only one bug.
[X] I have read and understood the contribution guidelines.

Affected app version

1.1.3

Affected Android/Custom ROM version

Android 14 / GrapheneOS

Affected device model

Google Pixel 6a

How did you install the app?

F-Droid / IzzyOnDroid

Steps to reproduce the bug

Make sure you're using the Material You theme.
Select an image.
In the overflow menu, choose "copy to" or "move to".
Select the "Choose a folder" option.
Check the color of the directory info row.

Expected behavior

The background color of the directory info row matches the background color of the rest of the dialog.

Actual behavior

The background color of the directory info row is different than the background color of the rest of the dialog:

Screenshots/Screen recordings

No response

Additional information

No response

min7-i commented 5 months ago

Sorry, I forgot to mention that the correct color was used in previous versions, e.g. in 1.1.2. That's why I considered it a bug and not an enhancement.

connyduck commented 5 months ago

I think this is easily fixed by removing the background here, I'm just worried that might break something else. Which tests can I do to ensure that is not the case?

naveensingh commented 5 months ago

@connyduck that's for the first breadcrumb's background, not the whole horizontal scroll view. Applying the proper background color on the view itself should fix it but I haven't checked what's caused this in the first place. Probably some dependency update.

connyduck commented 5 months ago

@naveensingh No. Maybe it should be the background of the first breadcrumb, but if you look at the code closely you will notice that this is Resources which doesn't have a background setter, and neither has the outer this which is ItemBreadcrumbFirstBinding. So the background is actually set to the whole Breadcrumbs class.

This commit introduced the problem. Before the refactoring this actually was the view of the first breadcrumb.

Can I send a PR with a fix?

naveensingh commented 5 months ago

Right, I only had a quick peek.

It should be root.background instead of just background. You are welcome to raise a PR but do test the change once with different light/dark theme variants.

FossifyOrg / Gallery