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.

Sign up for GitHub

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

Jump to bottom

Replace Deprecated API Usages for EditImageActivity #12147

Closed
wants to merge 5 commits into from
Closed

Replace Deprecated API Usages for EditImageActivity #12147

wants to merge 5 commits into from

Conversation

alperozturk96
Copy link
Collaborator

@alperozturk96 alperozturk96 commented Nov 9, 2023 •
edited
Loading

  • Tests written, or not not needed

@alperozturk96
Copy link
Collaborator Author

/rebase

@nextcloud-command nextcloud-command force-pushed the refactor/update-deprecated-api-edit-image-activity branch from df27a79 to a6b3f2c Compare November 10, 2023 07:57
ZetaTom
ZetaTom requested changes Nov 13, 2023
View reviewed changes
Copy link
Collaborator

@ZetaTom ZetaTom left a comment

Choose a reason for hiding this comment

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

When using Android 11.0 on a Pixel 2 with button navigation enabled, these buttons overlap the image editor controls.

@alperozturk96
Copy link
Collaborator Author

@ZetaTom Updated

with_nav_bar

@alperozturk96
Copy link
Collaborator Author

/rebase

@alperozturk96
Update api usages
653eb1a 
Signed-off-by: alperozturk <[email protected]>
@alperozturk96
Shift layout if nav bar exist
55d4496 
Signed-off-by: alperozturk <[email protected]>
@alperozturk96
Use extensions
411a133 
Signed-off-by: alperozturk <[email protected]>
@nextcloud-command nextcloud-command force-pushed the refactor/update-deprecated-api-edit-image-activity branch from 5dac918 to 411a133 Compare November 14, 2023 09:54
@alperozturk96
Copy link
Collaborator Author

@ZetaTom Could you review this PR again?

@ZetaTom
Copy link
Collaborator

ZetaTom commented Nov 21, 2023

@alperozturk96, given the most recent version (411a133) the issue still persists on my machine.

image_editor.webm

@alperozturk96
Copy link
Collaborator Author

alperozturk96 commented Nov 21, 2023 •
edited
Loading

@ZetaTom Could you check again with latest commit (c48baf7) ?

fix2 fix1

@alperozturk96
Remove optional navbarcheck
677c80e 
Signed-off-by: alperozturk <[email protected]>
@alperozturk96
Merge master
c48baf7 
Signed-off-by: alperozturk <[email protected]>
@github-actions GitHub Actions
Copy link

Codacy

Lint

TypemasterPR
Warnings7474
Errors00

SpotBugs

CategoryBaseNew
Bad practice2626
Correctness7070
Dodgy code363363
Experimental22
Internationalization99
Malicious code vulnerability22
Multithreaded correctness99
Performance5858
Security1818
Total557557

@github-actions GitHub Actions
Copy link

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/12147.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

@ZetaTom ZetaTom mentioned this pull request Nov 27, 2023
Merged
1 task
ZetaTom
ZetaTom reviewed Nov 27, 2023
View reviewed changes
Copy link
Collaborator

@ZetaTom ZetaTom left a comment

Choose a reason for hiding this comment

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

There are still some UI inconsistencies left and these changes do not follow the recommended migration path.

I've opened a pull request of my own with my suggestions already implemented: #12204

master this PR
master pr

@alperozturk96
Copy link
Collaborator Author

Screenshot 2023-11-27 at 13 19 40

With #12204 this implementation control buttons overlap with image. Since new PR opened we can close this.

@tobiasKaminsky tobiasKaminsky deleted the refactor/update-deprecated-api-edit-image-activity branch October 7, 2024 05:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ZetaTom ZetaTom ZetaTom requested changes

@AndyScherzinger AndyScherzinger Awaiting requested review from AndyScherzinger

@tobiasKaminsky tobiasKaminsky Awaiting requested review from tobiasKaminsky

Assignees
No one assigned
Labels
4. to release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@alperozturk96 @ZetaTom