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

[Theme Switching] 🐛 Android - Distance - Mapbox remains in dark mode when the app is in light mode #33137

Closed
1 of 6 tasks
lanitochka17 opened this issue Dec 15, 2023 · 18 comments

Comments

@lanitochka17
Copy link

lanitochka17 commented Dec 15, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 1.4.13-2
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

Precondition: App is in light mode

  1. Launch New Expensify app
  2. Tap FAB icon > Request money
  3. Go to Distance
  4. Tap on the mapbox info icon

Expected Result:

The mapbox info pop-up is in light mode

Actual Result:

The mapbox info pop-up remains in dark mode

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6313913_1702605188405!Mapbox

View all open jobs on GitHub

@lanitochka17 lanitochka17 added the DeployBlockerCash This issue or pull request should block deployment label Dec 15, 2023
Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

Copy link

melvin-bot bot commented Dec 15, 2023

Triggered auto assignment to @jasperhuangg (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@jasperhuangg
Copy link
Contributor

I don't think we need to block deploy on this, gonna remove the label

@jasperhuangg jasperhuangg added Daily KSv2 Design and removed Hourly KSv2 DeployBlockerCash This issue or pull request should block deployment labels Dec 15, 2023
Copy link

melvin-bot bot commented Dec 15, 2023

Triggered auto assignment to @dannymcclain (Design), see these Stack Overflow questions for more details.

@jasperhuangg
Copy link
Contributor

Looping in Design, are we expecting this modal to also be in a light theme? cc @dannymcclain

@jasperhuangg
Copy link
Contributor

jasperhuangg commented Dec 15, 2023

This seems like a similar design problem to #33120 (comment)

cc @dubielzyk-expensify @grgia

@dannymcclain
Copy link
Contributor

@jasperhuangg definitely feels similar to that issue you linked. I think if we can update this for light mode, that would be ideal. But if for some reason we can't/won't/don't want to, I don't think it's the end of the world since this feels almost more like "mapbox modal" than it does Expensify UI.

@grgia grgia changed the title Android - Distance - Mapbox remains in dark mode when the app is in light mode [Theme Switching] 🐛 Android - Distance - Mapbox remains in dark mode when the app is in light mode Dec 15, 2023
@dubielzyk-expensify
Copy link
Contributor

Yeah, if not, I think we should just revert to using system defaults here so that the OS can take care of the colors. Given this is a mapbox specific thing, I think that's fine

@melvin-bot melvin-bot bot added the Overdue label Dec 17, 2023
@dannymcclain
Copy link
Contributor

Based on this thread in Slack, it looks like we've reached a decision to not add any custom styling to native popups and let the OS handle the theming. @grgia that sound right to you?

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Dec 18, 2023
Copy link

melvin-bot bot commented Dec 22, 2023

@dannymcclain, @jasperhuangg Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

Copy link

melvin-bot bot commented Dec 26, 2023

@dannymcclain, @jasperhuangg Still overdue 6 days?! Let's take care of this!

Copy link

melvin-bot bot commented Dec 28, 2023

@dannymcclain, @jasperhuangg 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

Copy link

melvin-bot bot commented Jan 1, 2024

@dannymcclain, @jasperhuangg 12 days overdue now... This issue's end is nigh!

@jasperhuangg
Copy link
Contributor

Bump @grgia on this comment

@melvin-bot melvin-bot bot removed the Overdue label Jan 2, 2024
@melvin-bot melvin-bot bot added the Overdue label Jan 4, 2024
Copy link

melvin-bot bot commented Jan 5, 2024

@dannymcclain, @jasperhuangg Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@jasperhuangg
Copy link
Contributor

Bumped @grgia via DM

@melvin-bot melvin-bot bot removed the Overdue label Jan 8, 2024
@grgia
Copy link
Contributor

grgia commented Jan 8, 2024

Yes that sounds correct, we can close this as I have a master issue for this here #33235

@melvin-bot melvin-bot bot added the Overdue label Jan 10, 2024
@dannymcclain
Copy link
Contributor

Closing per Georgia's comment! 🌂

@melvin-bot melvin-bot bot removed Overdue labels Jan 10, 2024
@dannymcclain dannymcclain reopened this Jan 10, 2024
@dannymcclain dannymcclain closed this as not planned Won't fix, can't repro, duplicate, stale Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants