New Room Preview facilities #2373

gnunicorn · 2024-11-15T17:43:02Z

You will now get the room preview shown (if the server supports it), with the ability to join the space, if you have the permissions to,

When you click on an Space / Chat Invite

preview-on-invite.mp4

When you try to access an object via Ref you don't have access to yet

unknown-object-via-boost.mp4

When you scan a QR code to an object you aren't in the space of yet

screen-20250119-125254.mp4

For additional convienience, we are also showing exactly what will be seen/shown/shared over the QR code to the other user, too:

To the reviewer:
Sorry, for again coming with such a large pull request. But in certain parts of the app (especially with really old code, like this joining rooms one), it's like the famous "pulling one thread and the entire thing unravels" ... and cascading changes are needed.

In particular these are (not the order they were committed):

The actual RoomPreview rust, ffi and dart support, obviously ;)
linking the room preview to room invites: 0b5f89a
a bigger refactor of ref-item to allow for opening the room preview and have people join and be forwarded from there (as shown above) in 753abc4
A minor update in the error handler to give us the option to show better, more specific access information if the server provides it for us: which is the case if we aren't allowed to see the room preview. This is all the textBuilder-changes on those ActerError-Widget in db8e099
Two big changes were done around joining rooms:
- the via-servers are now provided as a list on the dart side as well (as it should be) and we provide our main server in the ref-details for external sharing for proper support of that - which also brings some new rust API to generate lists of strings
- a refactor of the joinRoom-action to remove the forwarding-callback and instead use the future-syntax as we do in other actions as well, in f2036f9
- which also brings the - hopefully final - fix for showing a 404 after forwarding to a joined space in 26f9214

codecov · 2024-11-15T17:44:45Z

Codecov Report

Attention: Patch coverage is 0% with 126 lines in your changes missing coverage. Please review.

Project coverage is 28.84%. Comparing base (8d97615) to head (2d5d2cd).
Report is 14 commits behind head on main.

Files with missing lines	Patch %	Lines
native/acter/src/api/room/preview.rs	0.00%	95 Missing ⚠️
native/acter/src/api/utils.rs	0.00%	12 Missing ⚠️
native/core/src/events/common/object_reference.rs	0.00%	9 Missing ⚠️
native/acter/src/api/client.rs	0.00%	7 Missing ⚠️
native/acter/src/api/room.rs	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2373      +/-   ##
==========================================
- Coverage   28.88%   28.84%   -0.04%     
==========================================
  Files         680      695      +15     
  Lines       45422    46083     +661     
==========================================
+ Hits        13119    13292     +173     
- Misses      32303    32791     +488

Flag	Coverage Δ
integration-test	`39.58% <0.00%> (-0.25%)`	⬇️
unittest	`20.03% <0.00%> (+0.19%)`	⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…iews

gnunicorn · 2025-01-14T12:42:13Z

app/lib/common/toolkit/errors/util.dart

 enum ErrorCode {
  notFound,
+  forbidden,


Extended the Error types to be more specific and more precise and allow us to show a different error depending on the underlying code we were given by the server: most notably if we were forbidden access.

gnunicorn · 2025-01-14T12:42:39Z

app/lib/features/preview/widgets/room_preview.dart

+        error: error,
+        stack: stack,
+        textBuilder: (error, errCode) => switch (errCode) {
+          ErrorCode.forbidden =>


Showing different errors depending on the error indicated by the server.

gnunicorn · 2025-01-14T12:43:19Z

app/lib/common/utils/utils.dart

@@ -113,6 +113,10 @@ List<String> asDartStringList(FfiListFfiString data) {
  return data.toList().map((e) => e.toDartString()).toList();
 }

+extension FfiListFfiStringtoDart on FfiListFfiString {


made this more accessible via helper extension

gnunicorn · 2025-01-14T12:44:07Z

app/lib/features/chat/pages/sub_chats_page.dart

@@ -167,7 +169,7 @@ class SubChatsPage extends ConsumerWidget {
        [];
    final remoteSubchats =
        ref.watch(remoteChatRelationsProvider(spaceId)).valueOrNull ?? [];
-    final entries = [];
+    final List<(SpaceHierarchyRoomInfo?, String)> entries = [];


being specific in types allows the dart compiler to figure out the type below isn't correct.

gnunicorn · 2025-01-14T12:46:28Z

app/lib/features/deep_linking/widgets/item_preview_card.dart

+import 'package:flutter/material.dart';
+import 'package:phosphor_flutter/phosphor_flutter.dart';
+
+class ItemPreviewCard extends StatelessWidget {


Generic type that can render an item preview, as used internally by the ref-details or the uri-parsing entity

gnunicorn · 2025-01-14T12:53:40Z

app/lib/features/room/actions/join_room.dart

-  String? server,
-  Function(String)? forward,
-) async {
+Future<String?> joinRoom({


Several fixes and refactors in this one:

replace that bad old pattern of having a forward-function passed with a proper future based returning instead

allow to throw the error or return an null on failure

support for more than one via-server as the API now allows for that, too

fix the check of the room actually being available and accessible

gnunicorn · 2025-01-14T12:54:33Z

app/lib/features/room/actions/join_room.dart

+    final isSpace = await _ensureLoadedWithinTime(() async {
+      final room = await ref.refresh(maybeRoomProvider(roomId).future);
+      return room?.isJoined() == true ? room!.isSpace() : null;
+    });


this now properly checks for the room being available and accessible to the UI before continuing.

gnunicorn · 2025-01-14T12:55:28Z

app/lib/features/room/widgets/room_chip.dart

+    );
+  }
+
+  Widget buildPreview(BuildContext context) {


new feature to show the preview in the chat for a room we don't have access to

gnunicorn · 2025-01-14T12:56:25Z

app/lib/features/share/action/share_space_object_action.dart

      shareContentBuilder: shareContentBuilder,
    );
  }

+  Widget? qrCodeHeader(BuildContext context) {


Minor feature (and useful for debugging): show the to be shared information within the QRcode above the QRcode as well.

gnunicorn · 2025-01-14T12:57:30Z

app/test/features/deep_linking/parse_acter_uri_test.dart

tests updated for additional via-support that we always include now

kumarpalsinh25

Nice addition!!

Added few comments few none of that blocking for merge. PR changes looks good to me.

kumarpalsinh25 · 2025-01-21T05:33:16Z

app/lib/common/extensions/record_helpers.dart

+class AllHashed<T> {
+  final List<T> items;
+
+  AllHashed(this.items);
+
+  @override
+  int get hashCode => Object.hashAll(items);
+
+  @override
+  bool operator ==(Object other) {
+    if (other is AllHashed<T>) {
+      return items == other.items;
+    } else if (other is List<T>) {
+      return items == other;
+    }
+    return false;
+  }
+}


Not understanding this.

two list of items, even if they are all the same, will come out as "different" if they are different instances. This is used to ensure that https://github.com/acterglobal/a3/pull/2373/files#diff-b2a403334550e3f97cc376cf1228271526121ca2ecf70cf62e6c50ba41e69289R14 caches the input value and not keeps triggering for every render.

kumarpalsinh25 · 2025-01-21T05:38:16Z

app/lib/common/toolkit/errors/util.dart

+    } else if (errorStr.contains('[400 / UNKNOWN]')) {
+      return ErrorCode.unknown;
+    } else if (errorStr.contains('[403 / M_FORBIDDEN]')) {
+      return ErrorCode.forbidden;


errorStr will contains exact [400 / UNKNOWN]?

If not then how about making condition like if (errorStr.contains('400') || errorStr.contains('UNKNOWN'))

yes, this is exact on purpose.

kumarpalsinh25 · 2025-01-21T05:48:38Z

app/lib/features/deep_linking/types.dart

+  ObjectType? finalType() {
+    ObjectRef? cur = objectPath;
+    while (cur?.child != null) {
+      cur = cur?.child; // find the deepest object
+    }
+    return cur?.objectType;
+  }


Not understanding this code.

it traverses the object path of the reference type to give the most internal objects type.

the way this is constructed is that the actor URI allows unlimited deep linking. Think:

/tl/asdf/t/339390/c/2323 -> this is a reference to the comment 2323 of task 33930 of task list asdf. The way this is represented in the parse uri result is that every object with an object path can have an additional child which (going left to right) gives the next inner type. So this would be TaskList[id=asdf, child=Task[id=339390. child=Comment[id=2323]]]. In order to find out what the actual final object is (in this case "comment"), you need to traverse that tree. Which is what this code does: look for the most inner item that doesn't have any child (in the while) and then return its objectType.

kumarpalsinh25 · 2025-01-21T06:19:28Z

app/lib/features/deep_linking/widgets/reference_details_item.dart

+      headerInfo: Column(
+        crossAxisAlignment: CrossAxisAlignment.start,
+        children: [
+          Padding(
+            padding: EdgeInsets.symmetric(vertical: 8),
+            child: Text(
+              L10n.of(context).toAccess,
+              style: Theme.of(context).textTheme.bodyMedium,
+            ),
+          ),
+          ItemPreviewCard(
+            // showing the same item without the tap
+            title: refDetails.title(),
+            refType: typeFromRefDetails(refDetails),
+          ),
+          Padding(
+            padding: EdgeInsets.symmetric(vertical: 8),
+            child: Text(
+              L10n.of(context).needToBeMemberOf,
+              style: Theme.of(context).textTheme.bodyMedium,
+            ),
+          ),
+        ],
+      ),
+    );


Probably we can design better design for this. (Later part..)

Also can be generalised this code as I see this used couple of times..

it is already generalized some part. But I didn't get this much further without making things a lot more specific again ... Happy to go further though.

kumarpalsinh25 · 2025-01-21T06:21:09Z

app/lib/features/deep_linking/widgets/reference_details_item.dart

+  final bool tapEnabled;
+  final EdgeInsets? margin;
+
+  const ReferenceDetailsItem({


What is the need of ReferenceDetailsItem as we have ItemPreviewCard. Can't we combined those in single widget?

kumarpalsinh25 · 2025-01-21T06:29:59Z

app/lib/features/preview/widgets/room_preview.dart

+  Widget roomHeader(RoomPreview preview) => Consumer(
+        builder: (context, ref, child) => ListTile(
+          leading: ActerAvatar(
+            options: AvatarOptions(ref.watch(roomPreviewAvatarInfo(query))),


Don't understand this query thing.

query needs roomId and via-server listing, which is combined as query record.

kumarpalsinh25 · 2025-01-21T06:32:05Z

app/lib/features/room/actions/join_room.dart

+    try {
+      final res = await callback();
+      if (res != null) {
+        return res;
+      }
+    } catch (e) {
+      if (throwError) rethrow;


Don't understand need of this change.

this code was meant to repeatedly try to call the function until a timeout occured. But that wasn't the case if there was an exception raised in the inner code, it would just fail immediately instead of retrying. This fixes the retry behaviour to what we really wanted to have in the first place.

New Room Preview facilities

99b5caf

gnunicorn added 4 commits November 18, 2024 14:30

Starting out with preview builder

bc11c77

Add avatar loading support

8d6c425

actions

83783fb

Merge remote-tracking branch 'origin/main' into ben-room-previews

81944e0

gnunicorn force-pushed the ben-room-previews branch from 36036c8 to 81944e0 Compare November 22, 2024 10:20

gnunicorn added 2 commits November 22, 2024 12:44

Improved error messages text builder

db8e099

Fix room preview query facilities

19bfd33

gnunicorn mentioned this pull request Nov 22, 2024

Deep linking setup #2360

Merged

19 tasks

gnunicorn added 17 commits December 4, 2024 12:02

Merge remote-tracking branch 'origin/main' into ben-room-previews

c298c3f

Post-merge test fixes

4f0b82f

remove unnecessary provider

62e5577

Merge branch 'ben-check-singleton-in-release-only' into ben-room-prev…

b6ab9a0

…iews

Merge remote-tracking branch 'origin/main' into ben-room-previews

43d5ccb

Merge remote-tracking branch 'origin/main' into ben-room-previews

f4aebfc

More preview for invites

55a420a

Merge remote-tracking branch 'origin/main' into ben-room-previews

8ab10ee

expose missing api fn

bed4c20

Link RoomPreview to shared objects

0b5f89a

Merge remote-tracking branch 'origin/main' into ben-room-previews

76c3b8a

Common preview item card for items from unknown rooms

753abc4

Join buttons on preview

9b57e61

remove join room forwarding param

f2036f9

Fixing up join and forward to selected item

26f9214

Fix room preview tests

7842344

Merge remote-tracking branch 'origin/main' into ben-room-previews

703d7f6

gnunicorn commented Jan 14, 2025

View reviewed changes

gnunicorn added 2 commits January 14, 2025 16:52

remove comment

d3e9516

Merge remote-tracking branch 'origin/main' into ben-room-previews

6f967c8

Adding changelog

018d2d6

gnunicorn requested review from bitfriend, gtalha07 and kumarpalsinh25 and removed request for bitfriend and gtalha07 January 19, 2025 12:58

gnunicorn marked this pull request as ready for review January 19, 2025 12:59

gnunicorn requested review from bitfriend and gtalha07 January 20, 2025 10:57

kumarpalsinh25 approved these changes Jan 21, 2025

View reviewed changes

gnunicorn added 3 commits January 21, 2025 16:05

Merge remote-tracking branch 'origin/main' into ben-room-previews

013111b

Merge remote-tracking branch 'origin/main' into ben-room-previews

97ec8e1

remove double line in faulty merge

5bce726

gnunicorn enabled auto-merge January 21, 2025 16:44

gnunicorn disabled auto-merge January 22, 2025 11:53

gnunicorn added the merge post release only Merge this only after the new release has been cut label Jan 22, 2025

alwaysClientProvider is now async

2d5d2cd

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Room Preview facilities #2373

New Room Preview facilities #2373

gnunicorn commented Nov 15, 2024 •

edited

Loading

codecov bot commented Nov 15, 2024 •

edited

Loading

gnunicorn Jan 14, 2025

gnunicorn Jan 14, 2025

gnunicorn Jan 14, 2025

gnunicorn Jan 14, 2025

gnunicorn Jan 14, 2025

gnunicorn Jan 14, 2025

gnunicorn Jan 14, 2025

gnunicorn Jan 14, 2025

gnunicorn Jan 14, 2025

gnunicorn Jan 14, 2025

kumarpalsinh25 left a comment

kumarpalsinh25 Jan 21, 2025

gnunicorn Jan 21, 2025

kumarpalsinh25 Jan 21, 2025

gnunicorn Jan 21, 2025

kumarpalsinh25 Jan 21, 2025

gnunicorn Jan 21, 2025

kumarpalsinh25 Jan 21, 2025

gnunicorn Jan 21, 2025

kumarpalsinh25 Jan 21, 2025

kumarpalsinh25 Jan 21, 2025

gnunicorn Jan 21, 2025

kumarpalsinh25 Jan 21, 2025

gnunicorn Jan 21, 2025

New Room Preview facilities #2373

Are you sure you want to change the base?

New Room Preview facilities #2373

Conversation

gnunicorn commented Nov 15, 2024 • edited Loading

When you click on an Space / Chat Invite

When you try to access an object via Ref you don't have access to yet

When you scan a QR code to an object you aren't in the space of yet

codecov bot commented Nov 15, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kumarpalsinh25 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gnunicorn commented Nov 15, 2024 •

edited

Loading

codecov bot commented Nov 15, 2024 •

edited

Loading