Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Move TypingView into the timeline as another item #7565

Merged
merged 2 commits into from
Nov 10, 2022
Merged

Move TypingView into the timeline as another item #7565

merged 2 commits into from
Nov 10, 2022

Conversation

jmartinesp
Copy link
Member

@jmartinesp jmartinesp commented Nov 10, 2022 •
edited
Loading

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

Creates a new TypingItem that contains a TypingView so it's not part of the timeline.

Motivation and context

Closes #7564 .

Screenshots / GIFs

Before After
old_typing_view.webm new_typing_item.webm

Tests

  • Open a room shared with another account you can sign in into.
  • Write from that other account.

Tested devices

  • Physical
  • Emulator
  • OS version(s): 9

Checklist

@jmartinesp jmartinesp requested a review from a team November 10, 2022 11:56
@jmartinesp jmartinesp self-assigned this Nov 10, 2022
@jmartinesp jmartinesp requested review from Florian14 and removed request for a team November 10, 2022 11:56
bmarty
bmarty approved these changes Nov 10, 2022
View reviewed changes
Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

Just a quick remark, else LGTM.

Putting the typing item as part of the timeline was the expected design at the beginning. Thanks!

...or/src/main/java/im/vector/app/features/home/room/detail/timeline/TimelineEventController.kt Outdated
@@ -334,6 +337,10 @@ class TimelineEventController @Inject constructor(
.setVisibilityStateChangedListener(Timeline.Direction.FORWARDS)
.addWhenLoading(Timeline.Direction.FORWARDS)

val typingUsers = partialState.roomSummary?.typingUsers.orEmpty()
val typingItem = TypingItem_().id("typing_view").avatarRenderer(avatarRenderer).users(typingUsers)
add(typingItem)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe do not add the typing item if there is a forward loading item. So add the item only if showingForwardLoader is false.

@jmartinesp jmartinesp force-pushed the feature/add-typing-view-as-footer-to-timeline branch from ae64dcb to 91b07d9 Compare November 10, 2022 16:12
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@jmartinesp jmartinesp enabled auto-merge (squash) November 10, 2022 17:18
@jmartinesp jmartinesp merged commit 008432a into develop Nov 10, 2022
@jmartinesp jmartinesp deleted the feature/add-typing-view-as-footer-to-timeline branch November 10, 2022 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bmarty bmarty bmarty approved these changes

@Florian14 Florian14 Awaiting requested review from Florian14 Florian14 was automatically assigned from element-hq/element-android-reviewers

Assignees

@jmartinesp jmartinesp

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make TypingView part of the timeline so it doesn't take extra space above the composer
2 participants
@jmartinesp @bmarty