Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Refactor DefaultLoadBitmapUseCase to receive orientation as argument #27290

Closed
MatthewTighe opened this issue Oct 5, 2022 · 5 comments · Fixed by #27937, fork-house/fenix#14, nathanmkaya/fenix#108 or Leland-Takamine/fenix#159
Assignees
Milestone

Comments

@MatthewTighe
Copy link
Contributor

MatthewTighe commented Oct 5, 2022

The WallpaperUseCases.DefaultLoadBitmapUseCase currently accepts a Context as an argument. It then determines the current device orientation inline in order to load the correct (landscape/portrait) bitmap for a wallpaper.

By removing the Context parameter and replacing it with the orientation, the function should be more testable and maintainable. It is also the only Context-dependent part of the updated Use Cases and is a copied holdover from the legacy versions, so it will mean we should be able to completely remove Context from WallpaperUseCases after #26968

┆Issue is synchronized with this Jira Task

@github-actions github-actions bot added the needs:triage Issue needs triage label Oct 5, 2022
@MatthewTighe MatthewTighe removed the needs:triage Issue needs triage label Oct 5, 2022
@mbaimuratov
Copy link
Contributor

Will be happy to contribute here!

One question:

I guess I also need to provide filesDir as a param, since we are getting it from context too:
val file = File(context.filesDir, path)
right?

@MatthewTighe
Copy link
Contributor Author

Great, thanks for helping out!

I guess I also need to provide filesDir as a param, since we are getting it from context too:
val file = File(context.filesDir, path)
right?

That's correct. The filesDir is already available to the top-level WallpaperUseCases, so it should just be a matter of passing it down.

@mbaimuratov
Copy link
Contributor

Great, thanks for helping out!

I guess I also need to provide filesDir as a param, since we are getting it from context too:
val file = File(context.filesDir, path)
right?

That's correct. The filesDir is already available to the top-level WallpaperUseCases, so it should just be a matter of passing it down.

Then, I guess, I could contribute here :)

@oyeraghib
Copy link

@mbaimuratov are you still working on with this ?

PrashantSaroj added a commit to PrashantSaroj/fenix that referenced this issue Nov 6, 2022
@github-actions github-actions bot added the eng:reopen-for-qa Reopens and tags the issue for QA needed when the issue is merged label Nov 6, 2022
@PrashantSaroj
Copy link
Contributor

Hey @MatthewTighe, I would like to work on this issue. I have created this PR #27730 . How do I test if my changes are correct?

@github-actions github-actions bot removed the eng:reopen-for-qa Reopens and tags the issue for QA needed when the issue is merged label Nov 7, 2022
PrashantSaroj added a commit to PrashantSaroj/fenix that referenced this issue Nov 7, 2022
PrashantSaroj added a commit to PrashantSaroj/fenix that referenced this issue Nov 20, 2022
…ceive orientation as and files directory as argument
MatthewTighe pushed a commit to MatthewTighe/fenix that referenced this issue Nov 21, 2022
…ceive orientation and files directory as argument
@github-actions github-actions bot added eng:reopen-for-qa Reopens and tags the issue for QA needed when the issue is merged and removed eng:reopen-for-qa Reopens and tags the issue for QA needed when the issue is merged labels Nov 21, 2022
@mergify mergify bot closed this as completed in #27937 Nov 22, 2022
mergify bot pushed a commit that referenced this issue Nov 22, 2022
@github-actions github-actions bot added this to the 109 milestone Nov 22, 2022
JohanLorenzo pushed a commit to mozilla-mobile/firefox-android that referenced this issue Feb 14, 2023
…pUseCase to receive orientation and files directory as argument
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.