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.

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

Use a UITableView component for the timeline. #349

Merged
merged 10 commits into from
Dec 5, 2022
Merged

Conversation

pixlwave
Copy link
Member

@pixlwave pixlwave commented Dec 2, 2022

This PR replaces the SwiftUI timeline with one based upon UITableView. The cells are configured with a UIHostingConfiguration so are still SwiftUI based. This gives us the ability to update the scroll position after changing items but before the changes are rendered meaning the scroll jumps should be fixed.

  • The back pagination threshold is now double the height of the table view
  • The table will keep back paginating to fill the threshold.
  • Items are only added/updated when the table isn't scrolling as updating the scroll position kills any inertia. (Further pagination is blocked until items are added from a pending pagination).
  • Conforms timeline items to Hashable for use in the diffable data source.
  • TimelineView has been merged into RoomScreen (happy to revert this if it doesn't make sense).

Todo

  • The syncing indicator still needs a ProgressView to match the figma (future PR).
  • Add UI tests so we notice if any desired behaviours are broken (future PR).
Back.pagination.mp4
Small Timeline
Simulator Screen Shot - iPhone 14 - 2022-12-02 at 12 41 42

@github-actions
Copy link

github-actions bot commented Dec 2, 2022

Warnings
⚠️ This pull request seems relatively large. Please consider splitting it into multiple smaller ones.
⚠️ Some of the commits are missing ticket numbers. Please consinder using them for better tracking.

Generated by 🚫 Danger Swift against 2469f89

@codecov
Copy link

codecov bot commented Dec 2, 2022

Codecov Report

Base: 34.41% // Head: 33.36% // Decreases project coverage by -1.04% ⚠️

Coverage data is based on head (e5062dc) compared to base (9415bd3).
Patch coverage: 7.65% of modified lines in pull request are covered.

❗ Current head e5062dc differs from pull request most recent head 2469f89. Consider uploading reports for the commit 2469f89 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #349      +/-   ##
===========================================
- Coverage    34.41%   33.36%   -1.05%     
===========================================
  Files          232      233       +1     
  Lines        15059    15537     +478     
  Branches      9188     9424     +236     
===========================================
+ Hits          5182     5184       +2     
- Misses        9728    10198     +470     
- Partials       149      155       +6     
Flag Coverage Δ
unittests 13.69% <0.00%> (-0.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...mentX/Sources/Generated/Strings+Untranslated.swift 54.54% <ø> (-12.13%) ⬇️
.../Sources/Screens/RoomScreen/RoomScreenModels.swift 50.00% <0.00%> (-50.00%) ⬇️
...es/Screens/RoomScreen/View/TimelineTableView.swift 0.00% <0.00%> (ø)
...entX/Sources/Services/Media/MediaSourceProxy.swift 0.00% <0.00%> (ø)
...line/TimeLineItemContent/MessageTimelineItem.swift 0.00% <ø> (ø)
...TimelineItems/EventBasedTimelineItemProtocol.swift 93.54% <ø> (ø)
...ne/TimelineItems/Items/EmoteRoomTimelineItem.swift 0.00% <ø> (ø)
...imelineItems/Items/EncryptedRoomTimelineItem.swift 0.00% <ø> (ø)
...ine/TimelineItems/Items/FileRoomTimelineItem.swift 0.00% <ø> (ø)
...ne/TimelineItems/Items/ImageRoomTimelineItem.swift 0.00% <ø> (ø)
... and 22 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@pixlwave pixlwave marked this pull request as ready for review December 2, 2022 18:13
@pixlwave pixlwave requested a review from stefanceriu December 5, 2022 11:47
Copy link
Member

@stefanceriu stefanceriu left a comment

Choose a reason for hiding this comment

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

This looks so promising, hope we can squash all the little edge cases and get something really solid out of it 👏

@pixlwave pixlwave requested a review from stefanceriu December 5, 2022 15:06
@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 5, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Member

@stefanceriu stefanceriu left a comment

Choose a reason for hiding this comment

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

lgtm 👍 👏

@pixlwave pixlwave enabled auto-merge (squash) December 5, 2022 15:17
@pixlwave pixlwave merged commit 3c893ba into develop Dec 5, 2022
@pixlwave pixlwave deleted the doug/timeline branch December 5, 2022 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants