Add URL previews as a Labs feature #4790

pixlwave · 2021-09-06T14:57:53Z

Adds support for URL previews, fixing #888. Depends on matrix-org/matrix-ios-kit#893 and matrix-org/matrix-ios-sdk#1220.

The feature is disabled by default with a Labs setting to enable it.
Previews are only generated for unencrypted rooms, with MatrixKit not detecting links for encrypted events.
There is a Core Data store to cache preview data for 24 hours and to remember when a preview has been closed.
A urlPreviewData property has been added to RoomBubbleCellData which is filled with the preview data once fetched, along with a showURLPreview property to avoid having to hit Core Data too often.
Whilst loading events the cell data is split to ensure that links only exist in the last bubble component to simplify timeline display.

Updated:

…ew model. Changes to RoomDataSource still to come.

Implement close button and store the action in Core Data. Hide the preview image view when no image is received. Remove line breaks in description text.

Make the preview manager a singleton (passing in the MXSession to functions). Fix tests. PreviewManager → URLPreviewManager URLPreviewViewData → URLPreviewData URLPreviewCache → URLPreviewStore

…reviews

Using a temporary position in the settings screen whilst waiting for feedback.

github-actions · 2021-09-06T15:27:14Z

📱 Scan the QR code below to install the build for this PR.
🔒 This build is for internal testing purpose. Only devices listed in the ad-hoc provisioning profile can install Element Alpha.

If you can't scan the QR code you can install the build via this link: https://i.diawi.com/M4Lpog

niquewoodhouse

Don't think the loading preview text is needed, there's a spinner already in there
The setting could say underneath the switch "will only show in unencrypted rooms" just to set that expectation

Update bubbleTableView's content offset when a preview above the bottom most visible cell changes the height of the table's content.

Riot/Modules/Room/CellData/RoomBubbleCellData.m

Riot/Modules/Room/DataSources/RoomDataSource.m

Riot/Managers/URLPreviews/ClosedURLPreview.swift

Riot/Managers/URLPreviews/URLPreviewStore.swift

Rename store.store(_:) to store.cache(_:).

langleyd

Don't have context on a lot of the code so just picked out some general stuff. Looks great though.

Riot/Managers/URLPreviews/ClosedURLPreview.swift

Riot/Managers/URLPreviews/URLPreviewData.swift

Riot/Managers/URLPreviews/URLPreviewManager.swift

Riot/Managers/URLPreviews/URLPreviewStore.swift

Riot/Modules/Room/CellData/RoomBubbleCellData.m

Riot/Modules/Room/Views/URLPreviews/URLPreviewView.swift

RiotTests/URLPreviewStoreTests.swift

Riot/Managers/URLPreviews/ClosedURLPreview.swift

SBiOSoftWhare · 2021-09-08T12:13:46Z

Riot/Managers/URLPreviews/URLPreviewCacheData.swift

+
+import CoreData
+
+extension URLPreviewCacheData {


I'm wondering if we will have more Core Data entities which prefix or suffix we can use to distinguish the managed object from the memory object. A convention like CDURLPreviewData for Core Data and URLPreviewData for the memory representation?

Riot/Managers/URLPreviews/URLPreviewManager.swift

Riot/Modules/Room/CellData/RoomBubbleCellData.m

SBiOSoftWhare · 2021-09-08T12:43:45Z

Riot/Modules/Room/DataSources/RoomDataSource.m

+                    NSURL *link = component.link;
+                    URLPreviewView *urlPreviewView;
+
+                    // Show a URL preview if the component has a link that should be previewed.
+                    if (link && RiotSettings.shared.roomScreenShowsURLPreviews && cellData.showURLPreview)
+                    {
+                        urlPreviewView = [URLPreviewView instantiate];
+                        urlPreviewView.preview = cellData.urlPreviewData;
+                        urlPreviewView.delegate = self;
+
+                        [temporaryViews addObject:urlPreviewView];
+
+                        if (!bubbleCell.tmpSubviews)
+                        {
+                            bubbleCell.tmpSubviews = [NSMutableArray array];
+                        }
+                        [bubbleCell.tmpSubviews addObject:urlPreviewView];
+
+                        urlPreviewView.translatesAutoresizingMaskIntoConstraints = NO;
+                        [bubbleCell.contentView addSubview:urlPreviewView];
+
+                        CGFloat leftMargin = RoomBubbleCellLayout.reactionsViewLeftMargin;
+                        if (roomBubbleCellData.containsBubbleComponentWithEncryptionBadge)
+                        {
+                            leftMargin+= RoomBubbleCellLayout.encryptedContentLeftMargin;
+                        }
+


- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath start to become huge, it will be time to make intermediate methods and factorize some code. We can do it in another PR if needed.

Riot/Modules/Room/DataSources/RoomDataSource.m

Riot/Modules/Room/RoomViewController.m

SBiOSoftWhare · 2021-09-08T12:59:40Z

Riot/Modules/Room/Views/BubbleCells/RoomIncomingTextMsgWithoutSenderInfoBubbleCell.m

+    if (RiotSettings.shared.roomScreenShowsURLPreviews && bubbleData && bubbleData.showURLPreview)
+    {
+        CGFloat height = [super heightForCellData:cellData withMaximumWidth:maxWidth];
+        return height + RoomBubbleCellLayout.urlPreviewViewTopMargin + [URLPreviewView contentViewHeightFor:bubbleData.urlPreviewData];
+    }
+
+    return [super heightForCellData:cellData withMaximumWidth:maxWidth];


We already have a mechanism in RoomBubbleCellData to compute the final height in - (void)addVerticalWhitespaceToString:(NSMutableAttributedString *)attributedString forEvent:(NSString *)eventId. It can be weird to manipulate sizes in two different places.

This comment applies to all cell classes with a custom + (CGFloat)heightForCellData:(MXKCellData *)cellData withMaximumWidth:(CGFloat)maxWidth

Thank you, that's so much neater! I really disliked doing this in the cells individually.

I've reverted this change for the current build as it doesn't automatically update when a preview changes from loading to loaded, and again from loaded to closed.

I don't want to hold back the release for this, so will look at it next.

URLPreviewManager becomes URLPreviewService. addVerticalWhitespaceToString used instead of heightForCellData multiple times. All newline characters removed.

URLPreviewCacheData becomes URLPreviewData in the model with a class name of URLPreviewDataMO ClosedURLData becomes URLPreviewUserData in the model with a class name of URLPreviewUserDataMO

This reverts commit ad618b4.

pixlwave added 19 commits July 21, 2021 15:14

Begin adding link detection to RoomBubbleCellData.

eacb8a4

Merge branch 'develop' into doug/888_add_url_previews

661b536

Merge branch 'develop' into doug/888_add_url_previews

22382e9

Add PreviewManger with Core Data cache and a URLPreviewView with a vi…

29758d1

…ew model. Changes to RoomDataSource still to come.

Add comments about the un-sanitized URL.

5f598a9

Load and store URLPreviewViewData in RoomBubbleCellData.

59e5416

Implement close button and store the action in Core Data. Hide the preview image view when no image is received. Remove line breaks in description text.

Refactoring and tidy up.

a81ebbd

Make the preview manager a singleton (passing in the MXSession to functions). Fix tests. PreviewManager → URLPreviewManager URLPreviewViewData → URLPreviewData URLPreviewCache → URLPreviewStore

Use stack views for layout.

042eb8e

Update layout for text only previews.

9fb13b7

Show an activity indicator until the preview has loaded.

1831b61

Merge remote-tracking branch 'origin/develop' into doug/888_add_url_p…

4924110

…reviews

Ensure correct font is used.

6a5b12a

Add setting to disable URL previews.

2e04123

Using a temporary position in the settings screen whilst waiting for feedback.

Fix edits to previewable links not working.

be83d8e

Hide the loading state on error.

80f8cc6

Break-up cell data after a link even if the new event isn't a message.

7db81cc

Fix reactions beneath URL previews.

434657e

Clear the URL preview manager's store when clearing caches.

cf3733c

Fix potentially redundant table reloading.

55df930

pixlwave linked an issue Sep 6, 2021 that may be closed by this pull request

URL previews! #888

Closed

pixlwave requested a review from niquewoodhouse September 6, 2021 15:27

niquewoodhouse reviewed Sep 7, 2021

View reviewed changes

pixlwave added 5 commits September 7, 2021 14:49

Observe URL preview update notification in RoomViewController.

7448ca1

Update bubbleTableView's content offset when a preview above the bottom most visible cell changes the height of the table's content.

Fix unsatisfiable constraints messages.

1c7adf0

Move url preview setting under labs section.

0094add

Remove "Loading preview..." label.

4ad0416

Fix settings toggle not enabled.

c007bc5

This was referenced Sep 7, 2021

Add MXURLPreview and a previewForURL method to MXRestClient matrix-org/matrix-ios-sdk#1220

Merged

Add link property to MXKRoomBubbleComponent and parse events for links. matrix-org/matrix-ios-kit#893

Merged

Add changelog entry.

08d548c

pixlwave changed the title ~~Add URL previews~~ Add URL previews as a Labs feature Sep 7, 2021

pixlwave commented Sep 7, 2021

View reviewed changes

Riot/Modules/Room/CellData/RoomBubbleCellData.m Show resolved Hide resolved

pixlwave commented Sep 7, 2021

View reviewed changes

Riot/Modules/Room/DataSources/RoomDataSource.m Show resolved Hide resolved

pixlwave commented Sep 7, 2021

View reviewed changes

Riot/Managers/URLPreviews/ClosedURLPreview.swift Outdated Show resolved Hide resolved

pixlwave commented Sep 7, 2021

View reviewed changes

Riot/Managers/URLPreviews/URLPreviewStore.swift Outdated Show resolved Hide resolved

pixlwave added 2 commits September 7, 2021 17:51

Merge branch 'develop' into doug/888_add_url_previews

1c7cef5

Add more docs and comments.

ea14ed9

Rename store.store(_:) to store.cache(_:).

SBiOSoftWhare self-requested a review September 8, 2021 09:29

langleyd self-requested a review September 8, 2021 10:51

langleyd reviewed Sep 8, 2021

View reviewed changes

SBiOSoftWhare reviewed Sep 8, 2021

View reviewed changes

pixlwave added 3 commits September 8, 2021 15:10

Update for PR feedback.

24afc7a

URLPreviewManager becomes URLPreviewService. addVerticalWhitespaceToString used instead of heightForCellData multiple times. All newline characters removed.

Rename Core Data objects.

206017c

URLPreviewCacheData becomes URLPreviewData in the model with a class name of URLPreviewDataMO ClosedURLData becomes URLPreviewUserData in the model with a class name of URLPreviewUserDataMO

Revert height computation for now.

2656746

SBiOSoftWhare approved these changes Sep 8, 2021

View reviewed changes

pixlwave added 2 commits September 8, 2021 16:24

Add matrix.to to firstURLDetectionIgnoredHosts.

ad618b4

Revert "Add matrix.to to firstURLDetectionIgnoredHosts."

dfb63e1

This reverts commit ad618b4.

pixlwave merged commit 0e64b60 into develop Sep 8, 2021

pixlwave deleted the doug/888_add_url_previews branch September 8, 2021 16:19

pixlwave mentioned this pull request Sep 8, 2021

Resolves a couple of missed comments on URL previews #4805

Merged

pixlwave mentioned this pull request Sep 23, 2021

Use addVerticalWhitespaceToString: for URL Previews #4896

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add URL previews as a Labs feature #4790

Add URL previews as a Labs feature #4790

pixlwave commented Sep 6, 2021 •

edited

Loading

github-actions bot commented Sep 6, 2021 •

edited

Loading

niquewoodhouse left a comment •

edited

Loading

langleyd left a comment

SBiOSoftWhare Sep 8, 2021

SBiOSoftWhare Sep 8, 2021

SBiOSoftWhare Sep 8, 2021

pixlwave Sep 8, 2021

pixlwave Sep 8, 2021 •

edited

Loading


		import CoreData

		extension URLPreviewCacheData {

Add URL previews as a Labs feature #4790

Add URL previews as a Labs feature #4790

Conversation

pixlwave commented Sep 6, 2021 • edited Loading

github-actions bot commented Sep 6, 2021 • edited Loading

niquewoodhouse left a comment • edited Loading

Choose a reason for hiding this comment

langleyd left a comment

Choose a reason for hiding this comment

SBiOSoftWhare Sep 8, 2021

Choose a reason for hiding this comment

SBiOSoftWhare Sep 8, 2021

Choose a reason for hiding this comment

SBiOSoftWhare Sep 8, 2021

Choose a reason for hiding this comment

pixlwave Sep 8, 2021

Choose a reason for hiding this comment

pixlwave Sep 8, 2021 • edited Loading

Choose a reason for hiding this comment

pixlwave commented Sep 6, 2021 •

edited

Loading

github-actions bot commented Sep 6, 2021 •

edited

Loading

niquewoodhouse left a comment •

edited

Loading

pixlwave Sep 8, 2021 •

edited

Loading