Add expand/collapse UI for reactions #1249

langleyd · 2023-07-05T11:07:24Z

What's in this PR

Adds a CollapsibleFlowLayout for controlling the layout
Adds tests for this layout and some mocks for testing layouts generally
~~Improves the rendering of the reaction buttons which were not pixel perfect~~ Already done on develop.
Adds the UI for the expand collapse buttons ~~including the count of hidden items in the collapsed state~~.

I removed the count of items on the collapse button as it required watching the the frames with GeometryReader which can cause infinite loops. As you need:

Layout the views to figure out how many are hidden
Then update the button with the count
Which causes the layout to be triggered again. And on and on...

The custom layout api was introduced to avoid this kind of use of GeometryReader as apple detail here at WWDC. I encountered the loops when testing so I think it's probably best to keep things simple/reliable.

What it looks like

Todo

Remove the third party FlowLayout (SwiftUI-Flow)
Update unit and ui tests
Fix translations, imported from Localazy

- Adds a CollapsibleFlowLayout for controlling the layout - Adds tests for this layout and some mocks for testing layouts generally - Improves the rendering of the reaction buttons which were not pixel perfect - Adds the UI for the expand collapse buttons including the count of hidden items in the collapsed state.

github-actions · 2023-07-05T11:08:41Z

	Warnings
⚠️	This pull request seems relatively large. Please consider splitting it into multiple smaller ones.
⚠️	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/Screens/RoomScreen/RoomScreenViewModel.swift#L425 - SwiftLint rule 'function_body_length' did not trigger a violation in the disabled region. Please remove the disable command. (`superfluous_disable_command`)
⚠️	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 aa7cabd

codecov · 2023-07-05T11:21:11Z

Codecov Report

Patch coverage: 60.88% and project coverage change: +0.31 🎉

Comparison is base (c453cc0) 39.96% compared to head (d9bf448) 40.27%.

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

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1249      +/-   ##
===========================================
+ Coverage    39.96%   40.27%   +0.31%     
===========================================
  Files          428      429       +1     
  Lines        27244    27392     +148     
  Branches     14043    14096      +53     
===========================================
+ Hits         10887    11032     +145     
- Misses       16064    16065       +1     
- Partials       293      295       +2

Flag	Coverage Δ
unittests	`23.15% <60.44%> (+0.47%)`	⬆️

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

Impacted Files	Coverage Δ
...Sources/Other/SwiftUI/Layout/ViewFrameReader.swift	`75.00% <0.00%> (-15.91%)`	⬇️
...urces/Other/SwiftUI/Views/RoundedCornerShape.swift	`100.00% <ø> (ø)`
.../Sources/Screens/RoomScreen/RoomScreenModels.swift	`51.21% <ø> (ø)`
...een/View/Style/TimelineItemBubbledStylerView.swift	`21.50% <0.00%> (-0.20%)`	⬇️
...creen/View/Style/TimelineItemPlainStylerView.swift	`0.00% <0.00%> (ø)`
...s/Timeline/Fixtures/RoomTimelineItemFixtures.swift	`80.62% <ø> (ø)`
...meline/TimelineItems/RoomTimelineItemFactory.swift	`1.39% <0.00%> (-0.02%)`	⬇️
...een/View/Supplementary/TimelineReactionsView.swift	`12.64% <1.78%> (-17.36%)`	⬇️
...urces/Screens/RoomScreen/RoomScreenViewModel.swift	`42.23% <12.50%> (-0.53%)`	⬇️
.../CollapsibleFlowLayout/CollapsibleFlowLayout.swift	`94.73% <94.73%> (ø)`
... and 1 more

... and 7 files 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.

langleyd · 2023-07-05T11:49:25Z

There is a large enough amount of logic in this and layouts can be tricky, so probably worth waiting to merge this until after the release.

langleyd · 2023-07-05T15:44:01Z

Moving back to draft as it's not ready, noticed a couple of things. Happy to receive any early feedback thought :)

- Remove SwiftUI-Flow - Add strings by importing from Localyse - Remove count on expand button as requires GeometryReader and can cause loops - Don't use GeometryReader for hiding reactions with opacity(just put them way off screen for now) - Fix unit and UI tests

…into langleyd/reactions_expand_collapse

pixlwave

So great, really appreciate the level of documentation in this code, and tests for Layout are :chefs-kiss:

ElementX/Sources/Other/SwiftUI/Layout/ViewFrameReader.swift

ElementX/Sources/Other/CollapsibleFlowLayout/CollapsibleFlowLayout.swift

ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineReactionsView.swift

ElementX/Sources/Other/CollapsibleFlowLayout/CollapsibleFlowLayout.swift

- use synthesized inits - use rows rather than lines for naming flow layout - other naming improvements - reactions were already rendered in another ui test, removing my test on favour of those and updating the screenshots for those.

sonarcloud · 2023-07-10T13:33:13Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
5.1% Duplication

langleyd linked an issue Jul 5, 2023 that may be closed by this pull request

Display emoji reactions #783

Closed

3 tasks

langleyd removed a link to an issue Jul 5, 2023

Display emoji reactions #783

Closed

3 tasks

langleyd mentioned this pull request Jul 5, 2023

Display emoji reactions #783

Closed

3 tasks

Add comment for reactionsCollapsed binding.

3a41ea6

langleyd marked this pull request as ready for review July 5, 2023 11:49

langleyd requested a review from a team as a code owner July 5, 2023 11:49

langleyd requested review from pixlwave and removed request for a team July 5, 2023 11:49

langleyd marked this pull request as draft July 5, 2023 15:43

langleyd added 2 commits July 5, 2023 21:27

Merge branch 'develop' of https://github.com/vector-im/element-x-ios …

4afada8

…into langleyd/reactions_expand_collapse

langleyd marked this pull request as ready for review July 5, 2023 20:45

Merge branch 'develop' of https://github.com/vector-im/element-x-ios …

36b3ef4

…into langleyd/reactions_expand_collapse

pixlwave approved these changes Jul 7, 2023

View reviewed changes

Address PR comments

d9bf448

- use synthesized inits - use rows rather than lines for naming flow layout - other naming improvements - reactions were already rendered in another ui test, removing my test on favour of those and updating the screenshots for those.

langleyd enabled auto-merge (squash) July 10, 2023 13:06

Merge branch 'develop' into langleyd/reactions_expand_collapse

aa7cabd

langleyd merged commit 2cec185 into develop Jul 10, 2023
5 checks passed

langleyd deleted the langleyd/reactions_expand_collapse branch July 10, 2023 15:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add expand/collapse UI for reactions #1249

Add expand/collapse UI for reactions #1249

langleyd commented Jul 5, 2023 •

edited

Loading

github-actions bot commented Jul 5, 2023 •

edited

Loading

codecov bot commented Jul 5, 2023 •

edited

Loading

langleyd commented Jul 5, 2023

langleyd commented Jul 5, 2023

pixlwave left a comment

sonarcloud bot commented Jul 10, 2023

Add expand/collapse UI for reactions #1249

Add expand/collapse UI for reactions #1249

Conversation

langleyd commented Jul 5, 2023 • edited Loading

What's in this PR

What it looks like

Todo

github-actions bot commented Jul 5, 2023 • edited Loading

codecov bot commented Jul 5, 2023 • edited Loading

Codecov Report

langleyd commented Jul 5, 2023

langleyd commented Jul 5, 2023

pixlwave left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Jul 10, 2023

langleyd commented Jul 5, 2023 •

edited

Loading

github-actions bot commented Jul 5, 2023 •

edited

Loading

codecov bot commented Jul 5, 2023 •

edited

Loading