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

Add a QuickViewFragment and buttons for it #2680

Draft
wants to merge 7 commits into
base: release/3.7
Choose a base branch
from

Conversation

EmmanuelMess
Copy link
Member

@EmmanuelMess EmmanuelMess commented Jul 15, 2021

Description

Issue tracker

Related #2092

Automatic tests

  • Added test cases

Manual tests

  • Done

Build tasks success

Successfully running following tasks on local:

  • ./gradlew assembledebug
  • ./gradlew spotlessCheck

@EmmanuelMess
Copy link
Member Author

@VishalNehra 12 function limit? Most of these are short functions, please increase limit.

@EmmanuelMess EmmanuelMess marked this pull request as ready for review July 15, 2021 15:10
@VishalNehra
Copy link
Member

Done, please retrigger the build

@EmmanuelMess
Copy link
Member Author

Done, please retrigger the build

Done

@VishalNehra VishalNehra marked this pull request as draft July 18, 2021 12:17
@EmmanuelMess
Copy link
Member Author

EmmanuelMess commented Jul 19, 2021

@VishalNehra issues are black history icon in appbar and monetization?

@EmmanuelMess
Copy link
Member Author

@VishalNehra why is this stuck it doesn't even change behaviour on release builds.

@VishalNehra
Copy link
Member

@VishalNehra why is this stuck it doesn't even change behaviour on release builds.

Sorry, what's stuck?

@EmmanuelMess
Copy link
Member Author

@VishalNehra why is this stuck it doesn't even change behaviour on release builds.

Sorry, what's stuck?

The PR, it is marked as draft.

@VishalNehra
Copy link
Member

@EmmanuelMess Oh, you mean for review? We discussed on this right. We decided on few changes. Plus we wanted to migrate to a plugin for the same.

@EmmanuelMess
Copy link
Member Author

@EmmanuelMess Oh, you mean for review? We discussed on this right. We decided on few changes. Plus we wanted to migrate to a plugin for the same.

How are we going to migrate to the plugin?

What changes, I need a list here so I can work on them.

@VishalNehra
Copy link
Member

@EmmanuelMess Sure.. from the top of my head... It'll be a simple list of fragments / activities able to handle image / music intents in the plugin plugin application.. and from amaze it'll just be a conventional open file intent call, where our plugin activity will show up.

So I see no changes in amaze right now (maybe later we show a small one time demo of how they can purchase our plugin to quickly see the image when they press on it)

@EmmanuelMess
Copy link
Member Author

Sure.. from the top of my head... It'll be a simple list of fragments / activities able to handle image / music intents in the plugin plugin application.. and from amaze it'll just be a conventional open file intent call, where our plugin activity will show up.

I won't implement that, as I said, that would remove the benefits of having a fast system to show how files look in the app, and I believe it will not go with the features a file manager should provide.

So I see no changes in amaze right now (maybe later we show a small one time demo of how they can purchase our plugin to quickly see the image when they press on it)

I need to know how the division of code should be done, what parts go on the plug in and what goes in Amaze. I would prefer as much as possible to be implemented in Amaze. The more systematic the choosing of code the better.

@VishalNehra
Copy link
Member

VishalNehra commented Aug 1, 2021

I won't implement that, as I said, that would remove the benefits of having a fast system to show how files look in the app, and I believe it will not go with the features a file manager should provide.

Oh, don't worry, it'll be fast view in the app only as a form of plugin. The activity can be a small dialog instead of a full screen image view. I would like to quote an app shared by @TranceLove to get an idea of what I mean.

I need to know how the division of code should be done, what parts go on the plug in and what goes in Amaze. I would prefer as much as possible to be implemented in Amaze. The more systematic the choosing of code the better.

With the above approach the plugin will handle everything through intent, I don't think there would be a need to put any image viewer code in Amaze (neither should we, as you yourself stated, it's not responsibility of a FM).

Lemme know the confusion, we can discuss on the group.

@EmmanuelMess
Copy link
Member Author

I don't think there would be a need to put any image viewer code in Amaze (neither should we, as you yourself stated, it's not responsibility of a FM).

There is IMO the need for fast visualization of arbitrary files on Amaze, especially for files on the cloud or via ssh, to exceed would be to add more gallery features like cropping or intents IMO.

There is no image viewer code in Amaze, it is almost all managed by Glide, and then there is a generic view hierarchy for all quick views.

With the above approach the plugin will handle everything through intent,

Can we implement the intent management correctly without extensive code writing for managing different sources, like internal storage, ssh, ftp, etc?

@EmmanuelMess
Copy link
Member Author

EmmanuelMess commented Aug 1, 2021

Lemme know the confusion, we can discuss on the group.

IMO it seems easier for me to keep track of the conversation points on GitHub.

@VishalNehra
Copy link
Member

There is IMO the need for fast visualization of arbitrary files on Amaze, especially for files on the cloud or via ssh, to exceed would be to add more gallery features like cropping or intents IMO.

The fast visualization can be done with plugin as well. We can have caching as well for remote files if we want. And plugin will be easier to monetize.

@VishalNehra
Copy link
Member

to exceed would be to add more gallery features like cropping or intents IMO.

It'll be published as plug-in, so no need to add those all features initially.
Later if we want we can always expand feature set.

@EmmanuelMess
Copy link
Member Author

EmmanuelMess commented Aug 1, 2021

It'll be published as plug-in, so no need to add those all features initially.

IMO those can never be added as it would consume dev time from other more file manager related features and not be part of what a file manager should do.

@EmmanuelMess
Copy link
Member Author

The fast visualization can be done with plugin as well. We can have caching as well for remote files if we want. And plugin will be easier to monetize.

I understand the need to have a plugin, but how should code separation be done?

Also, this does not have intents currently for loading files, will intents work correctly with files from the cloud?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants