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

Filex to open files #2058

Merged
merged 15 commits into from
Aug 14, 2024
Merged

Filex to open files #2058

merged 15 commits into from
Aug 14, 2024

Conversation

gnunicorn
Copy link
Contributor

@gnunicorn gnunicorn commented Aug 13, 2024

built on top of #2048 this adds facilities to open or share files.

Specifically, this replaces the existing DownloadButton with a new ShareFileButton (from the newly added files feature) that will open a bottom up drawer for the file in question and allows the user to chose between

  1. opening the file (with anything the system provides for it),
  2. share the file (via the system provided sharing facilities), or
  3. save it to disk (not supported on Android because of FilePicker crashes on android #1803 ).

See it in action:

share-and-open.mp4

Fixes #1663.
Additionally this adds the Phosphor-Icons library as it had a nicer icon and we wanted to start using that anyways...

@gnunicorn gnunicorn added the bug Something isn't working label Aug 13, 2024
Copy link
Contributor

@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.

Useful addition to File management.

],
return IconButton(
icon: PhosphorIcon(PhosphorIcons.shareFat()),
onPressed: () => onShareEvent(calendarEvent),
);
}

Future<void> onShareEvent(CalendarEvent event) async {
Copy link
Contributor

Choose a reason for hiding this comment

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

How about putting ShareEvent on action folder?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

could do, but it is single use case ... so I'll keep it here for now.

crossAxisAlignment: CrossAxisAlignment.start,
children: [
header ?? const SizedBox.shrink(),
if (header != null) const SizedBox(height: 16.0),
Copy link
Contributor

Choose a reason for hiding this comment

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

if (header != null) const SizedBox(height: 16.0),
Not understanding purpose of this code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if there is a header, we want some padding between it and the items.

Comment on lines +38 to +39
final List<Widget>? beforeOptions;
final List<Widget>? afterOptions;
Copy link
Contributor

Choose a reason for hiding this comment

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

  final List<Widget>? beforeOptions;
  final List<Widget>? afterOptions;

Can I have more details about this options?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

like what? it allows you to inject further options in the list... either at the top or bottom. I ended up not using it, I can also remove it...

label: Text(lang.shareFile),
icon: PhosphorIcon(PhosphorIcons.shareNetwork()),
),
if (!Platform.isAndroid) // crashes on Android for some reason ...
Copy link
Contributor

Choose a reason for hiding this comment

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

// crashes on Android for some reason ...
This is probably due to new File system permission introduced in latest android OS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

possibly. if you have an idea for a fix, the bug is reported here #1803

Copy link
Contributor

Choose a reason for hiding this comment

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

If it is the case what I am thinking then it require bit of discussion on the same. I will cross check details and update you.

@@ -133,6 +133,7 @@ dependencies:
shake_detector:
path: ../packages/shake_detector
open_filex: ^4.5.0
phosphor_flutter: ^2.1.0
Copy link
Contributor

Choose a reason for hiding this comment

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

In recent past, I have explore this package and check with Acter app. Here is the my findings.

Advantages:

  • Phosphor have better look and feel compare to Atlas
  • Phosphor have more number of icons compare to Atlas

Disadvantages:

  • Phosphor have less variety for single icon category

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Phosphor have less variety for single icon category

I don't understand what that means. Can you elaborate?

Copy link
Contributor

@kumarpalsinh25 kumarpalsinh25 Aug 14, 2024

Choose a reason for hiding this comment

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

Lets example of icons with Email category. Atlas have more variety in single category.

Icons (1)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah. gotcha ... not sure that matter too much though...

Copy link
Contributor

Choose a reason for hiding this comment

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

Screen.Recording.2024-08-14.at.4.09.58.PM.mov

@gnunicorn gnunicorn merged commit 0db59a3 into main Aug 14, 2024
17 checks passed
@gnunicorn gnunicorn deleted the ben-filex-open branch August 14, 2024 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working flutter
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Can not view files/pdf's in acter chat
2 participants