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

Improvement in refDetail and Space Object sharing enhancements #2472

Merged
merged 21 commits into from
Jan 3, 2025

Conversation

kumarpalsinh25
Copy link
Contributor

@kumarpalsinh25 kumarpalsinh25 commented Dec 31, 2024

This PR contains general code improve in Space Object Sharing code and enhancement of sharing options by enabling External Sharing Options.

Screen.Recording.2025-01-03.at.10.36.06.AM.mov

Also, User invite share screen now used common sharing UI-UX.

Screen.Recording.2025-01-03.at.10.37.43.AM.mov

Copy link

codecov bot commented Dec 31, 2024

Codecov Report

Attention: Patch coverage is 57.89474% with 96 lines in your changes missing coverage. Please review.

Project coverage is 27.91%. Comparing base (de5bf23) to head (fc355da).
Report is 27 commits behind head on main.

Files with missing lines Patch % Lines
native/acter/src/api/deep_linking.rs 59.15% 58 Missing ⚠️
native/acter/src/api/stories.rs 0.00% 18 Missing ⚠️
native/acter/src/api/news.rs 11.11% 16 Missing ⚠️
native/acter/src/testing.rs 75.00% 2 Missing ⚠️
native/acter/src/api/attachments.rs 50.00% 1 Missing ⚠️
native/acter/src/api/common.rs 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2472      +/-   ##
==========================================
+ Coverage   27.76%   27.91%   +0.15%     
==========================================
  Files         660      660              
  Lines       44697    44775      +78     
==========================================
+ Hits        12410    12499      +89     
+ Misses      32287    32276      -11     
Flag Coverage Δ
integration-test 38.33% <57.89%> (+0.35%) ⬆️
unittest 19.10% <ø> (-0.01%) ⬇️

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.

Copy link
Contributor Author

@kumarpalsinh25 kumarpalsinh25 left a comment

Choose a reason for hiding this comment

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

Added few comment for code explanation.

@@ -3,7 +3,7 @@ import 'dart:io';
import 'package:appcheck/appcheck.dart';
import 'package:flutter_riverpod/flutter_riverpod.dart';

enum ExternalApps { whatsApp, telegram, signal }
enum ExternalApps { whatsApp, whatsBusiness, telegram, signal }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added WhatsBusiness app direct share option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just moved file to different folder.

required BuildContext context,
required String text,
}) async {
if (Platform.isAndroid) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Direct share to Signal App only available for android platform with Explicit Intent.

For iOS users, given better guide about sharing to Signal.

Copy link
Contributor

Choose a reason for hiding this comment

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

We usually do the path of only showing features if they are available - if this isn't possible on iOS rather than showing some error messages, the button should not show up.

Comment on lines -22 to -27
typedef SpaceObjectDetails = ({
String spaceId,
ObjectType objectType,
String objectId,
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Get rid of SpaceObjectDetails and use RefDetails param directly to sharing options.

Comment on lines 25 to 26
Future<String> Function()? shareContentBuilder,
Future<FileDetails> Function()? fileDetailContentBuilder,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ExternalLink and FileSharing will be based on the callback function which make data accessing only when user interact will specific option.

Comment on lines -214 to -243
NewsReferencesType? getNewsRefTypeFromObjType(ObjectType objectType) {
return switch (objectType) {
ObjectType.pin => NewsReferencesType.pin,
ObjectType.calendarEvent => NewsReferencesType.calendarEvent,
ObjectType.taskList => NewsReferencesType.taskList,
_ => null,
};
}

Future<RefDetails?> getRefDetails({
required WidgetRef ref,
required SpaceObjectDetails objectDetails,
}) async {
final objectId = objectDetails.objectId;
switch (objectDetails.objectType) {
case ObjectType.pin:
final sourcePin = await ref.watch(pinProvider(objectId).future);
return await sourcePin.refDetails();
case ObjectType.calendarEvent:
final sourceEvent =
await ref.watch(calendarEventProvider(objectId).future);
return await sourceEvent.refDetails();
case ObjectType.taskList:
final sourceTaskList =
await ref.watch(taskListProvider(objectId).future);
return await sourceTaskList.refDetails();
default:
return null;
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Say Good bye to unwanted code as refDetails is now directly available from class param.

Comment on lines -9 to -14
final GestureTapCallback? onTapCopy;
final GestureTapCallback? onTapQr;
final GestureTapCallback? onTapSignal;
final GestureTapCallback? onTapWhatsApp;
final GestureTapCallback? onTapTelegram;
final GestureTapCallback? onTapMore;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Get rid of this callbacks as all of the options will be used when shareContent is available.

Comment on lines 446 to 465
final refDetails = await event.refDetails();
final internalLink = refDetails.generateInternalLink(true);
if (context.mounted) {
openShareSpaceObjectDialog(
context: context,
spaceObjectDetails: (
spaceId: event.roomIdStr(),
objectType: ObjectType.calendarEvent,
objectId: widget.calendarId,
),
fileDetails: (
file: File(icalPath),
mimeType: 'text/calendar',
),
refDetails: refDetails,
internalLink: internalLink,
shareContentBuilder: () => refDetails.generateExternalLink(),
fileDetailContentBuilder: () async {
final filename =
event.title().replaceAll(RegExp(r'[^A-Za-z0-9_-]'), '_');
final tempDir = await getTemporaryDirectory();
final icalPath = join(tempDir.path, '$filename.ics');
event.icalForSharing(icalPath);
return (
file: File(icalPath),
mimeType: 'text/calendar',
);
},
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

General code optimisation based on the above mentioned changes.

Comment on lines +114 to +116
return ExternalShareOptions(
qrContent: qrContent,
shareContentBuilder: () async => shareContent,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used ExternalShareOptions common widget here also for user invites. This make sharing option more generic and removal of code duplication.

Comment on lines 72 to 76
await openShareSpaceObjectDialog(
context: context,
refDetails: refDetails,
internalLink: internalLink,
shareContentBuilder: () => refDetails.generateExternalLink(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the only change, other changes showing in this file are only related Code Format.

@kumarpalsinh25 kumarpalsinh25 changed the title Improvement in refDetail and Object sharing Improvement in refDetail and Space Object sharing enhacements Jan 3, 2025
@kumarpalsinh25 kumarpalsinh25 changed the title Improvement in refDetail and Space Object sharing enhacements Improvement in refDetail and Space Object sharing enhancements Jan 3, 2025
@kumarpalsinh25 kumarpalsinh25 marked this pull request as ready for review January 3, 2025 05:12
Copy link
Contributor

@gnunicorn gnunicorn left a comment

Choose a reason for hiding this comment

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

But shouldn't we also close the share-dialog when someone clicked any of the things?

app/lib/common/widgets/share/action/shareTo.dart Outdated Show resolved Hide resolved
required BuildContext context,
required String text,
}) async {
if (Platform.isAndroid) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We usually do the path of only showing features if they are available - if this isn't possible on iOS rather than showing some error messages, the button should not show up.

@kumarpalsinh25
Copy link
Contributor Author

But shouldn't we also close the share-dialog when someone clicked any of the things?

Due to common usage of ExternalShareOptions widget in Share user invite code, I removed Navigator.pop(context); from Share Dialog as it was closing share invite screen also.

But ya after changes related to shareContentBuilder suggestion, now it is possible to managed navigation independently in Share Dialog.

Did required changes and pushed the code.

@kumarpalsinh25 kumarpalsinh25 merged commit e11211d into main Jan 3, 2025
21 of 22 checks passed
@kumarpalsinh25 kumarpalsinh25 deleted the kumar/ref-details-improvements branch January 3, 2025 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Recently Done
Development

Successfully merging this pull request may close these issues.

2 participants