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 feature to open an external PDF with kDrive using kDrive PDF viewer #1179

Merged
merged 68 commits into from
Apr 10, 2024

Conversation

tevincent
Copy link
Contributor

@tevincent tevincent commented Jan 31, 2024

depends on #1270

Add PreviewPDFActivity to give the possibility to the user to open a PDF on his phone with kDrive

kdrive_default_reader_pdf.webm

@tevincent tevincent requested a review from a team as a code owner January 31, 2024 14:00
@tevincent tevincent marked this pull request as draft January 31, 2024 14:00
@tevincent tevincent marked this pull request as ready for review January 31, 2024 14:02
@tevincent tevincent marked this pull request as draft February 1, 2024 07:27
@tevincent tevincent force-pushed the kdrive_as_defaut_pdf_viewer_app branch 3 times, most recently from 237a9f4 to 2e9bd4d Compare February 27, 2024 13:39
@Infomaniak Infomaniak deleted a comment from sonarqubecloud bot Feb 27, 2024
@tevincent tevincent marked this pull request as ready for review February 27, 2024 15:18
@tevincent tevincent force-pushed the kdrive_as_defaut_pdf_viewer_app branch 2 times, most recently from 4554342 to fcbab03 Compare February 28, 2024 09:08
@tevincent tevincent force-pushed the kdrive_as_defaut_pdf_viewer_app branch from e4c9f64 to c3d1d1f Compare March 8, 2024 08:44
@LunarX
Copy link
Contributor

LunarX commented Mar 13, 2024

The nav bar is of the wrong color, and is it normal that the UI doesn't hide when you click on the center of the screen like inside the app? Also I can't seem to be able entirely slide down the bottom sheet.

Maybe check if there's a way to filter-out our own app from the "open in" action. Google seems to be doing so with their drive app.

Also, did you check the issue reported by SonarCloud?

@tevincent tevincent force-pushed the kdrive_as_defaut_pdf_viewer_app branch from d154eaa to 26f7463 Compare March 14, 2024 15:14
@KevinBoulongne KevinBoulongne added the feature A new functionality is added to the product label Mar 15, 2024
@tevincent tevincent marked this pull request as draft March 15, 2024 13:18
@tevincent tevincent force-pushed the kdrive_as_defaut_pdf_viewer_app branch 5 times, most recently from 31416c3 to 7ca2a63 Compare March 18, 2024 13:32
@tevincent tevincent marked this pull request as ready for review March 18, 2024 13:35
Copy link
Contributor

@LunarX LunarX left a comment

Choose a reason for hiding this comment

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

Does "send a copy" still makes sens as a wording when sharing the file?

Also I got this error when opening any preview from within the app:

FATAL EXCEPTION: main
Process: com.infomaniak.drive, PID: 20933
kotlin.UninitializedPropertyAccessException: lateinit property bottomSheetBehavior has not been initialized
	at com.infomaniak.drive.ui.fileList.preview.PreviewSliderFragment.setupWindowInsetsListener$lambda$21$lambda$20(PreviewSliderFragment.kt:255)
	at com.infomaniak.drive.ui.fileList.preview.PreviewSliderFragment.$r8$lambda$ZdZOV44Tay1G4Ct21o1EmSpiRDM(Unknown Source:0)
	at com.infomaniak.drive.ui.fileList.preview.PreviewSliderFragment$$ExternalSyntheticLambda6.onApplyWindowInsets(Unknown Source:4)
	at androidx.core.view.ViewCompat$Api21Impl$1.onApplyWindowInsets(ViewCompat.java:5233)
	at android.view.View.dispatchApplyWindowInsets(View.java:11309)
	at android.view.ViewGroup.dispatchApplyWindowInsets(ViewGroup.java:7320)
	at androidx.core.view.ViewCompat$Api20Impl.dispatchApplyWindowInsets(ViewCompat.java:5975)
	at androidx.core.view.ViewCompat.dispatchApplyWindowInsets(ViewCompat.java:2960)
	at androidx.fragment.app.FragmentContainerView.dispatchApplyWindowInsets(FragmentContainerView.kt:218)
	at androidx.core.view.ViewCompat$Api20Impl.dispatchApplyWindowInsets(ViewCompat.java:5975)
	at androidx.core.view.ViewCompat.dispatchApplyWindowInsets(ViewCompat.java:2960)
	at androidx.fragment.app.FragmentContainerView.dispatchApplyWindowInsets(FragmentContainerView.kt:218)
	at android.view.ViewGroup.newDispatchApplyWindowInsets(ViewGroup.java:7345)
	at android.view.ViewGroup.dispatchApplyWindowInsets(ViewGroup.java:7327)
	at android.view.ViewGroup.newDispatchApplyWindowInsets(ViewGroup.java:7345)
	at android.view.ViewGroup.dispatchApplyWindowInsets(ViewGroup.java:7327)
	at android.view.ViewGroup.newDispatchApplyWindowInsets(ViewGroup.java:7345)
	at android.view.ViewGroup.dispatchApplyWindowInsets(ViewGroup.java:7327)
	at android.view.ViewGroup.newDispatchApplyWindowInsets(ViewGroup.java:7345)
	at android.view.ViewGroup.dispatchApplyWindowInsets(ViewGroup.java:7327)
	at android.view.ViewGroup.newDispatchApplyWindowInsets(ViewGroup.java:7345)
	at android.view.ViewGroup.dispatchApplyWindowInsets(ViewGroup.java:7327)
	at android.view.ViewGroup.newDispatchApplyWindowInsets(ViewGroup.java:7345)
	at android.view.ViewGroup.dispatchApplyWindowInsets(ViewGroup.java:7327)
	at android.view.ViewRootImpl.dispatchApplyInsets(ViewRootImpl.java:2291)
	at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:2525)
	at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:1952)
	at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:8171)
	at android.view.Choreographer$CallbackRecord.run(Choreographer.java:972)
	at android.view.Choreographer.doCallbacks(Choreographer.java:796)
	at android.view.Choreographer.doFrame(Choreographer.java:731)
	at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:957)
	at android.os.Handler.handleCallback(Handler.java:938)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:223)
	at android.app.ActivityThread.main(ActivityThread.java:7656)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947)

Copy link
Contributor

@LunarX LunarX left a comment

Choose a reason for hiding this comment

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

  1. Does "send a copy" still makes sens as a wording when sharing the file?

  2. Also the comportement when hidding the UI is still not the same as when I open the pdf inside the app. Why do the statusbar/navbar not hide when clicking in the center of the preview? Couldn't we have put in common between the two activities to keep the same behavior?

  3. Also, I don't know if this branch introduced the issue, but when viewing a PDF inside the app, when hidding the UI, the pdf scrollbar appears and when showing the UI, the scrollbar hides

@tevincent
Copy link
Contributor Author

tevincent commented Mar 19, 2024

@LunarX I changed the "Send a copy" wording after discussing with Joris. Maybe you can talk to him about that.

I have fixed the behavior to match what we have in kDrive PDF reader.

I tried to put everything I can in common between the default PDF reader and what we have in the app based on the current architecture of the app. Let me know what do you think.

default-pdf-reader-kDrive.webm

@tevincent tevincent force-pushed the kdrive_as_defaut_pdf_viewer_app branch from ca110cb to d13102f Compare April 8, 2024 13:25
@tevincent tevincent force-pushed the kdrive_as_defaut_pdf_viewer_app branch from 061c935 to 68b54ff Compare April 9, 2024 13:03
@Infomaniak Infomaniak deleted a comment from sonarqubecloud bot Apr 9, 2024
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@LunarX LunarX merged commit fa5eefd into master Apr 10, 2024
4 checks passed
@LunarX LunarX deleted the kdrive_as_defaut_pdf_viewer_app branch April 10, 2024 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new functionality is added to the product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants