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

Closes #6915: Add addon installation confirmation dialog #6916

Merged
merged 1 commit into from
May 12, 2020

Conversation

psymoon
Copy link
Contributor

@psymoon psymoon commented May 8, 2020


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.

@psymoon psymoon requested review from csadilek and Amejia481 May 8, 2020 15:02
@codecov
Copy link

codecov bot commented May 8, 2020

Codecov Report

Merging #6916 into master will increase coverage by 0.00%.
The diff coverage is 76.10%.

Impacted file tree graph

@@            Coverage Diff             @@
##             master    #6916    +/-   ##
==========================================
  Coverage     77.06%   77.06%            
- Complexity     4836     4853    +17     
==========================================
  Files           650      651     +1     
  Lines         23803    23917   +114     
  Branches       3489     3501    +12     
==========================================
+ Hits          18344    18432    +88     
- Misses         4009     4026    +17     
- Partials       1450     1459     +9     
Impacted Files Coverage Δ Complexity Δ
...ature/addons/ui/AddonInstallationDialogFragment.kt 76.10% <76.10%> (ø) 17.00 <17.00> (?)
...ort/migration/AbstractMigrationProgressActivity.kt 25.00% <0.00%> (-2.28%) 0.00% <0.00%> (ø%)
...ava/mozilla/components/support/migration/Result.kt 100.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
...components/support/sync/telemetry/SyncTelemetry.kt 87.09% <0.00%> (+1.38%) 41.00% <0.00%> (ø%)

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 16b7808...ce74add. Read the comment docs.

@psymoon psymoon force-pushed the installed-modal branch from 69edb3a to 2782bdc Compare May 8, 2020 17:26
Copy link
Contributor

@Amejia481 Amejia481 left a comment

Choose a reason for hiding this comment

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

Looks nice! 🦄

@Amejia481 Amejia481 self-assigned this May 11, 2020
@Amejia481 Amejia481 added the 🕵️‍♀️ needs review PRs that need to be reviewed label May 11, 2020
@psymoon psymoon force-pushed the installed-modal branch from 2782bdc to 534314e Compare May 11, 2020 16:42
Copy link
Contributor Author

@psymoon psymoon left a comment

Choose a reason for hiding this comment

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

The WebExtensionSupportTest CI failure:

[task 2020-05-11T16:51:54.697Z]     [Robolectric] mozilla.components.support.webextensions.WebExtensionSupportTest.reacts to new extension being installed: sdk=28; resources=binary
[task 2020-05-11T16:51:54.797Z]     I/MonitoringInstr: Instrumentation started!
[task 2020-05-11T16:51:54.797Z]     I/MonitoringInstr: Setting context classloader to 'org.robolectric.internal.bytecode.SandboxClassLoader@54c56106', Original: 'org.robolectric.internal.bytecode.SandboxClassLoader@54c56106'
[task 2020-05-11T16:51:54.897Z]   FAILURE

may be #6791.

Copy link
Contributor

@Amejia481 Amejia481 left a comment

Choose a reason for hiding this comment

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

Looks good just some nits!

@psymoon psymoon force-pushed the installed-modal branch from 534314e to 7a381ea Compare May 11, 2020 18:09
Copy link
Contributor

@csadilek csadilek 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. I have a suggestion about the API (the lambda) though which we should discuss.

@psymoon psymoon force-pushed the installed-modal branch 2 times, most recently from 49d2be0 to 4b7aacc Compare May 11, 2020 19:55
@psymoon psymoon requested a review from csadilek May 11, 2020 19:56
@psymoon psymoon force-pushed the installed-modal branch from 4b7aacc to ce74add Compare May 11, 2020 20:23
@psymoon
Copy link
Contributor Author

psymoon commented May 11, 2020

bors r=Amejia481, csadilek

bors bot pushed a commit that referenced this pull request May 11, 2020
6916: Closes #6915: Add addon installation confirmation dialog r=Amejia481,csadilek a=psymoon



Co-authored-by: Simon Chae <[email protected]>
@bors
Copy link

bors bot commented May 11, 2020

Timed out.

@psymoon
Copy link
Contributor Author

psymoon commented May 11, 2020

bors retry

bors bot pushed a commit that referenced this pull request May 11, 2020
6916: Closes #6915: Add addon installation confirmation dialog r=Amejia481,csadilek a=psymoon



6947: Closes #6791: Intermittent test failure in WebExtensionSupport r=psymoon a=csadilek

Basically, we dispatch a tab browser action to test that our action handler works, but that in turn changes the tab state triggering our flow again (see registerHandlersForNewSession). We can simply verify and capture the handlers first, then trigger actions to test them.

Co-authored-by: Simon Chae <[email protected]>
Co-authored-by: Christian Sadilek <[email protected]>
@bors
Copy link

bors bot commented May 11, 2020

This PR was included in a batch that timed out, it will be automatically retried

bors bot pushed a commit that referenced this pull request May 11, 2020
6916: Closes #6915: Add addon installation confirmation dialog r=Amejia481,csadilek a=psymoon



Co-authored-by: Simon Chae <[email protected]>
@bors
Copy link

bors bot commented May 12, 2020

Timed out.

@psymoon
Copy link
Contributor Author

psymoon commented May 12, 2020

bors retry

@bors
Copy link

bors bot commented May 12, 2020

Build succeeded:

@bors bors bot merged commit 7b7310d into mozilla-mobile:master May 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🕵️‍♀️ needs review PRs that need to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants