Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Issue #6827: Move Thumbnails into its own browser component #6835

Merged
merged 1 commit into from
May 1, 2020

Conversation

gabrielluong
Copy link
Member

@gabrielluong gabrielluong commented Apr 30, 2020

Fixes #6827.


As we work through adding a disk cache for thumbnails in #2754, it is becoming clear we should move the thumbnails feature into its own browser component. This is the first step in doing that by copying over ThumbnailsFeature.kt and ThumbnailsFeatureTest.kt over to BrowserThumbnails.

We also switched the Sample Browser to use the new BrowserThumbnails component. I have categorized this component as under development since #2754 will make it more of a real feature.

As the next step that I want to do is look into adding a deprecation warning for ThumbnailsFeature, but I didn't think it was necessary in this PR and could be made as a follow up.


Pull Request checklist

  • Quality: This PR builds and passes detekt/ktlint checks (A pre-push hook is recommended)
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Changelog: This PR includes a changelog entry or does not need one
  • Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features

After merge

  • Milestone: Make sure issues closed by this pull request are added to the milestone of the version currently in development.
  • Breaking Changes: If this is a breaking change, please push a draft PR on Reference Browser to address the breaking issues.

@codecov
Copy link

codecov bot commented Apr 30, 2020

Codecov Report

Merging #6835 into master will increase coverage by 0.15%.
The diff coverage is 78.57%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #6835      +/-   ##
============================================
+ Coverage     77.42%   77.58%   +0.15%     
- Complexity     4549     4764     +215     
============================================
  Files           607      632      +25     
  Lines         22671    23254     +583     
  Branches       3343     3424      +81     
============================================
+ Hits          17554    18042     +488     
- Misses         3717     3783      +66     
- Partials       1400     1429      +29     
Impacted Files Coverage Δ Complexity Δ
...components/browser/thumbnails/BrowserThumbnails.kt 78.57% <78.57%> (ø) 8.00 <8.00> (?)
...ents/browser/domains/DomainAutoCompleteProvider.kt 65.78% <0.00%> (ø) 12.00% <0.00%> (?%)
...mponents/browser/domains/autocomplete/Providers.kt 100.00% <0.00%> (ø) 0.00% <0.00%> (?%)
.../java/mozilla/components/browser/domains/Domain.kt 63.63% <0.00%> (ø) 4.00% <0.00%> (?%)
...omponents/service/fretboard/ExperimentsSnapshot.kt 100.00% <0.00%> (ø) 3.00% <0.00%> (?%)
...retboard/storage/flatfile/ExperimentsSerializer.kt 100.00% <0.00%> (ø) 6.00% <0.00%> (?%)
...mozilla/components/service/fretboard/Experiment.kt 100.00% <0.00%> (ø) 14.00% <0.00%> (?%)
...lla/components/service/fretboard/ValuesProvider.kt 100.00% <0.00%> (ø) 10.00% <0.00%> (?%)
.../components/service/fretboard/ExperimentPayload.kt 100.00% <0.00%> (ø) 9.00% <0.00%> (?%)
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c9ac872...d25b839. Read the comment docs.

@gabrielluong gabrielluong force-pushed the 6827 branch 2 times, most recently from 9a825a5 to 90df287 Compare May 1, 2020 16:47
Copy link
Contributor

@jonalmeida jonalmeida left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! Please put up a PR for r-b to fix the breaking APIs for when they land.

(land if CI is happy)

@gabrielluong
Copy link
Member Author

bors r+

@bors
Copy link

bors bot commented May 1, 2020

Build succeeded:

@bors bors bot merged commit a9eda72 into mozilla-mobile:master May 1, 2020
@gabrielluong gabrielluong deleted the 6827 branch May 1, 2020 23:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move Thumbnails into its own browser component
2 participants