room summary now has constant height #7145
Conversation
There was a problem hiding this comment.
I am thinking of fixing this issue since a long time, thanks for doing it! (I think I have a local branch with some attempt somewhere).
I was wondering if it would be possible to define a dimen resource for the cell height, which could be with unit sp, so that we ensure that the room item and the placeholder have the same height, and so we avoid regression. WDYT?
changelog.d/7079.bugfix
Outdated
| @@ -0,0 +1 @@ | |||
| Fix text margin in QR code view when no display name is set | |||
There was a problem hiding this comment.
now I wonder where I put the correct one 😅
|
|
||
| if (useSingleLineForLastEvent) { | ||
| holder.subtitleView.setLines(1) | ||
| } |
There was a problem hiding this comment.
Since items can be recycled, we need to add a else block here. But since the value is constant in the controller, I think this is fine.
| return roomSummaryItemFactory.create(item, roomChangeMembershipStates.orEmpty(), emptySet(), displayMode, listener) | ||
| return if (item == null) { | ||
| val host = this | ||
| RoomSummaryItemPlaceHolder_().apply { |
There was a problem hiding this comment.
Nit: roomSummaryItemPlaceHolder{ } could be used
| RoomSummaryItemPlaceHolder_().apply { | |
| roomSummaryItemPlaceHolder { |
There was a problem hiding this comment.
roomSummaryItemPlaceHolder adds model to collector, but here we need to return item instead, so we can't really use this inside buildItemModel method :(
| return roomSummaryItemFactory.create(item, roomChangeMembershipStates.orEmpty(), emptySet(), RoomListDisplayMode.ROOMS, listener) | ||
| return if (item == null) { | ||
| val host = this | ||
| RoomSummaryItemPlaceHolder_().apply { |
There was a problem hiding this comment.
Nit: roomSummaryItemPlaceHolder{ } could be used
| RoomSummaryItemPlaceHolder_().apply { | |
| roomSummaryItemPlaceHolder { |
There was a problem hiding this comment.
roomSummaryItemPlaceHolder adds model to collector, but here we need to return item instead, so we can't really use this inside buildItemModel method :(
|
|
||
| init { | ||
| val fontScale = fontScalePreferences.getResolvedFontScaleValue() | ||
| shouldUseSingleLine = fontScale.scale > FontScalePreferences.SCALE_LARGE |
There was a problem hiding this comment.
A tiny detail, but I would have used >=
| shouldUseSingleLine = fontScale.scale > FontScalePreferences.SCALE_LARGE | |
| shouldUseSingleLine = fontScale.scale >= FontScalePreferences.SCALE_LARGER |
There was a problem hiding this comment.
I don't feel ok checking equality of two floating-point numbers. We don't do any calculations here, so they should be the same, but I still concerned. 🤔
… to the first position
| * @param layoutManager - [LinearLayoutManager] of the recycler view, which displays items | ||
| * @param onItemUpdated - callback to be called, when observer detects event | ||
| */ | ||
| class FirstItemUpdatedObserver( |
There was a problem hiding this comment.
(you have to use @property instead of @param for class members documentation, in this case onItemUpdated )
fedrunov
added
the
Z-NextRelease
|
|
SonarCloud Quality Gate failed. |
Type of change
Content
Currently room summary item height is not constant - it can vary depending if there is enough text in the last message/event to go on a second line. This cause some flickering and jumping scrolls when placeholders are replaced with actual items. To mitigate this I've forced last event text view to have constant amount of lines (2 normally, reduced to 1 if font scale is bigger than "Large"). Also Placeholder's items had height defined in dp, which caused them to have wrong height if font size is scaled.
Motivation and context
closes #7079
Screenshots / GIFs
before changes (look for first room item's height):
After changes:
with font scaling (Huge):