[RUM-11291][FO][Part1/2]: Sample app integration - vitals screen refactoring#2820
Conversation
satween
changed the title
satween
changed the title
| } | ||
|
|
||
| @Suppress("MemberVisibilityCanBePrivate") | ||
| var lineOffset: Float = DEFAULT_LINE_OFFSET_DP.px |
There was a problem hiding this comment.
do we need the explicit types?
There was a problem hiding this comment.
I think for at least for the properties it's much easier to read such variables(primitives initialized from constants) inside non IDE environment (like on github). For the cases like with new object initialization it's okay to avoid explicit types IMO.
|
|
||
| init { | ||
| if (attrs != null) { | ||
| val a = context.obtainStyledAttributes(attrs, R.styleable.DividerView, 0, 0) |
There was a problem hiding this comment.
nit: should we make this variable name more descriptive? same for w and h later on
There was a problem hiding this comment.
changed a -> attributes, for w and h in onMeasure I think it's pretty clear what they mean
| var lineOffset: Float = DEFAULT_LINE_OFFSET_DP.px | ||
| set(value) { | ||
| field = value | ||
| invalidate() | ||
| } | ||
|
|
||
| @Suppress("MemberVisibilityCanBePrivate") | ||
| var textOffset: Float = DEFAULT_TEXT_OFFSET_DP.px | ||
| set(value) { | ||
| field = value | ||
| invalidate() | ||
| } |
There was a problem hiding this comment.
since those are changing the position, shouldn't requestLayout be called as well?
There was a problem hiding this comment.
invalidate should be enough here, as text offset doesn't change view bounds and DividerView is not a ViewGroup
ffbbe64
c286ec8 to
ffbbe64
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feature/featureoperations #2820 +/- ##
=============================================================
- Coverage 70.11% 70.10% -0.01%
=============================================================
Files 835 835
Lines 31418 31418
Branches 5272 5272
=============================================================
- Hits 22027 22025 -2
Misses 7916 7916
- Partials 1475 1477 +2 🚀 New features to boost your workflow:
|
satween
changed the title
satween
changed the title
6 participants
What does this PR do?
Making sample app's vital screen more compact in order to free up some space for FO buttons
Review checklist (to be filled by reviewers)