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

Extract radio group logic into helper #11493

Merged
merged 4 commits into from
Jul 7, 2020

Conversation

NotWoods
Copy link
Contributor

This helps remove code dedicated to combining radio buttons

Pull Request checklist

  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not
  • Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features. In addition, it includes a screenshot of a successful accessibility scan to ensure no new defects are added to the product.

After merge

  • Milestone: Make sure issues finished by this pull request are added to the milestone of the version currently in development.

To download an APK when reviewing a PR:

  1. click on Show All Checks,
  2. click Details next to "Taskcluster (pull_request)" after it appears and then finishes with a green checkmark,
  3. click on the "Fenix - assemble" task, then click "Run Artifacts".
  4. the APK links should be on the left side of the screen, named for each CPU architecture

@NotWoods NotWoods added the eng:tech-debt Engineering debt. Missing unit tests, etc. label Jun 11, 2020
@codecov-commenter
Copy link

codecov-commenter commented Jun 11, 2020

Codecov Report

Merging #11493 into master will decrease coverage by 1.22%.
The diff coverage is 14.28%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #11493      +/-   ##
============================================
- Coverage     22.12%   20.90%   -1.23%     
+ Complexity      725      681      -44     
============================================
  Files           377      374       -3     
  Lines         15098    15033      -65     
  Branches       1957     2034      +77     
============================================
- Hits           3341     3142     -199     
- Misses        11469    11607     +138     
+ Partials        288      284       -4     
Impacted Files Coverage Δ Complexity Δ
...sessioncontrol/viewholders/CollectionViewHolder.kt 0.00% <ø> (ø) 0.00 <0.00> (ø)
...ncontrol/viewholders/NoContentMessageViewHolder.kt 0.00% <ø> (ø) 0.00 <0.00> (ø)
...oncontrol/viewholders/TabInCollectionViewHolder.kt 0.00% <ø> (ø) 0.00 <0.00> (ø)
...ders/onboarding/OnboardingThemePickerViewHolder.kt 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...rding/OnboardingToolbarPositionPickerViewHolder.kt 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...boarding/OnboardingTrackingProtectionViewHolder.kt 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...trol/viewholders/topsites/TopSiteItemViewHolder.kt 0.00% <ø> (ø) 0.00 <0.00> (ø)
.../mozilla/fenix/onboarding/OnboardingRadioButton.kt 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...rg/mozilla/fenix/settings/CustomizationFragment.kt 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...rg/mozilla/fenix/settings/RadioButtonPreference.kt 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
... and 80 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 1ab5fe3...0c2004c. Read the comment docs.

@NotWoods NotWoods closed this Jun 12, 2020
@NotWoods NotWoods reopened this Jun 12, 2020
Copy link
Contributor

@ekager ekager left a comment

Choose a reason for hiding this comment

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

This is great!

@NotWoods NotWoods force-pushed the radio-group branch 3 times, most recently from a139d2a to 4f55779 Compare July 6, 2020 20:25
@NotWoods NotWoods merged commit 8e8e5ae into mozilla-mobile:master Jul 7, 2020
@NotWoods NotWoods deleted the radio-group branch July 7, 2020 00:09
@liuche liuche mentioned this pull request Jul 20, 2020
9 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
eng:tech-debt Engineering debt. Missing unit tests, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants