further touchup on kotlin conversion #154
Conversation
use more idiomatic language where possible, let-access, kotlin function calls, safe-access operators where possible fix endless loop during panelstate set remove companion object, unnecessary instance cost touchup javadoc where necessary fix several bugs caused by improper nullity access
| /** | ||
| * Default peeking out panel height | ||
| */ | ||
| private const val DEFAULT_PANEL_HEIGHT = 68 // dp; |
There was a problem hiding this comment.
Please move this back to companion object
| } | ||
| } | ||
| } | ||
|
|
||
| companion object { |
There was a problem hiding this comment.
Please add companion object again
There was a problem hiding this comment.
can you explain why? The companion is unnecessary here and adds an instance creation, wasting resources
| @@ -1140,7 +1234,13 @@ class SlidingUpPanelLayout @JvmOverloads constructor(context: Context, attrs: At | |||
| setAllChildrenVisible() | |||
| } | |||
|
|
|||
| override fun onViewPositionChanged(changedView: View?, left: Int, top: Int, dx: Int, dy: Int) { | |||
| override fun onViewPositionChanged( | |||
There was a problem hiding this comment.
This multi-line codestyle everywhere blows code up. I don't like it, but it's ok.
There was a problem hiding this comment.
I have no preference, we can revert that. Sorry I thought I have the kotlin default enabled for the auto-styling in the IDE, but you're saying single line is the default?
Something in the back of my head tells me there was an intention during kotlin language design to unify this better, I should google around a bit to check if there are global standards for this
There was a problem hiding this comment.
I keep not that much attention why, but on my side it mostly use a single line.
I've the flavor in my head of preventing huge files.
There was a problem hiding this comment.
was able to trim it down by a couple of line :)
please let me know what else can be done to be able to merge this :)
| override fun onOptionsItemSelected(item: MenuItem): Boolean { | ||
| when (item.itemId) { | ||
| R.id.action_toggle -> { | ||
| if (sliding_layout.panelState != PanelState.HIDDEN) { | ||
| sliding_layout.panelState = PanelState.HIDDEN | ||
| if (sliding_layout.getPanelState() != PanelState.HIDDEN) { |
There was a problem hiding this comment.
What's the point to move to getPanelState().
Why not using property access ?
I only get one more additional weak warning
There was a problem hiding this comment.
It looks like, that you are no more interested in applying here some more changes.
I will merge it and I'll will revoke some changes from you by myself.
I've no clue, why you removed property access, but I will revoke this
There was a problem hiding this comment.
Hi @hannesa2 I'm sorry I have two jobs currently, I am absolutely interested in this still, but I simply did not find time yet! I will look at the merge as soon as I can, please note that the variable panelState internally has a different name, this was causing the endless loop bug before, which I described in my first comment in this PR, it's important to understand which setter vs. field access is called by this method. But as I said, I will have a look at this soon. I'll build it in a clean way so that the IDE doesn't need to show warnings anymore.
Until then I ask for a bit more patience 🥺
demo app now also in working state
@hannesa2 are there any outstanding issues to merge this into master? please let me know, happy to help if I can