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

Sort reactions on the timeline by count and then timestamp and show the date on the summary view #1320

Merged
merged 5 commits into from
Jul 14, 2023

Conversation

langleyd
Copy link
Member

What's in this PR

  • Sort reaction aggregations on the timeline by count and then by most recent timestamp ascending(tagging new reactions on the end)
  • Show the timestamp on the summary view and sort descending(newest at the top)

What does it look like

- Sort reaction aggregations on the timeline by count and then by most recent timestamp ascending(tagging new reactions on the end)
- Show the timestamp on the summary view and sort descending(newest at the top)
@langleyd langleyd requested a review from a team as a code owner July 12, 2023 21:03
@langleyd langleyd requested review from pixlwave and removed request for a team July 12, 2023 21:03
@github-actions
Copy link

github-actions bot commented Jul 12, 2023

Warnings
⚠️ Please add a changelog.
⚠️ Some of the commits are missing ticket numbers. Please consider squashing all commits that don't have a tracking number.
⚠️ You seem to have made changes to views. Please consider adding screenshots.
⚠️

ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemFactory.swift#L40 - SwiftLint rule 'function_body_length' did not trigger a violation in the disabled region. Please remove the disable command. (superfluous_disable_command)

Generated by 🚫 Danger Swift against 5bbbcbd

@codecov
Copy link

codecov bot commented Jul 12, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.09 ⚠️

Comparison is base (bf91e97) 44.05% compared to head (5bbbcbd) 43.96%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1320      +/-   ##
===========================================
- Coverage    44.05%   43.96%   -0.09%     
===========================================
  Files          397      398       +1     
  Lines        26306    26309       +3     
  Branches     13387    13394       +7     
===========================================
- Hits         11588    11568      -20     
- Misses       14424    14446      +22     
- Partials       294      295       +1     
Flag Coverage Δ
unittests 22.46% <0.00%> (-0.03%) ⬇️

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

Impacted Files Coverage Δ
...lementX/Sources/Mocks/AggregatedReactionMock.swift 66.66% <ø> (-10.26%) ⬇️
ElementX/Sources/Other/Extensions/Duration.swift 0.00% <0.00%> (ø)
.../Other/UserIndicator/UserIndicatorController.swift 78.26% <0.00%> (ø)
...reen/View/Supplementary/ReactionsSummaryView.swift 0.00% <0.00%> (ø)
...s/Timeline/Fixtures/RoomTimelineItemFixtures.swift 100.00% <ø> (ø)
...line/TimelineItemContent/AggregratedReaction.swift 83.33% <0.00%> (-16.67%) ⬇️
...meline/TimelineItems/RoomTimelineItemFactory.swift 1.37% <0.00%> (-0.05%) ⬇️

... and 1 file with indirect coverage changes

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

Copy link
Member

@pixlwave pixlwave left a comment

Choose a reason for hiding this comment

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

LGTM 😎

- Fix ID case.
- Improve readability of components in body.
- Improve the comments that describe the sorting.
@langleyd langleyd enabled auto-merge (squash) July 14, 2023 14:07
@sonarcloud
Copy link

sonarcloud bot commented Jul 14, 2023

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 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@langleyd langleyd merged commit a44a60c into develop Jul 14, 2023
9 checks passed
@langleyd langleyd deleted the langleyd/reaction_timestamps branch July 14, 2023 16:18
@aringenbach
Copy link
Contributor

Fixes #1289

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.

3 participants