-
Notifications
You must be signed in to change notification settings - Fork 63
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
Improve SeekBarMapper #2037
Improve SeekBarMapper #2037
Conversation
@@ -151,6 +142,7 @@ internal abstract class BaseSeekBarWireframeMapperTest { | |||
@Mock | |||
lateinit var mockInternalLogger: InternalLogger | |||
|
|||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should it be left as commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call, this code should be deleted
// region Assertions | ||
fun assertThatBoundsAreCloseEnough(actual: MobileSegment.Wireframe, expected: MobileSegment.Wireframe) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// region Assertions | |
fun assertThatBoundsAreCloseEnough(actual: MobileSegment.Wireframe, expected: MobileSegment.Wireframe) { | |
// region Assertions | |
fun assertThatBoundsAreCloseEnough(actual: MobileSegment.Wireframe, expected: MobileSegment.Wireframe) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
if (actualCornerRadius != null && expectedCornerRadius != null) { | ||
assertThat(actualCornerRadius).isCloseTo(expectedCornerRadius, withinPercentage(5)) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// endregion
is missing, it seems
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@ForgeConfiguration(ForgeConfigurator::class) | ||
internal class LegacySeekBarWireframeMapperTest : BaseSeekBarWireframeMapperTest() { | ||
|
||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should it all be commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same, it's deleted
9a88e51
to
e086414
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2037 +/- ##
===========================================
+ Coverage 82.93% 83.17% +0.24%
===========================================
Files 491 491
Lines 17700 17697 -3
Branches 2683 2686 +3
===========================================
+ Hits 14678 14718 +40
+ Misses 2285 2238 -47
- Partials 737 741 +4
|
e086414
to
dadfe16
Compare
dadfe16
to
5459252
Compare
const val OPAQUE_ALPHA_VALUE: Int = MAX_ALPHA_VALUE | ||
|
||
/** The value corresponding to a 25% opaque Alpha. */ | ||
const val PARTIALLY_OPAQUE_ALPHA_VALUE: Int = 0x44 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a random mathematical comment here, 0x3f or 0x40 seems closer to 25% of opaque alpha.(Just wondering how 0x44 is calculated, although it has hardly any impact)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point thanks
5459252
to
a23e0e0
Compare
What does this PR do?
Improve SeekBarMapper and its unit tests: