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

#2445 - add Download/Delete buttons and display image size in the image viewer #2453

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

SebinSong
Copy link
Collaborator

closes #2445

[screenshot]

[screenshot in forced-color mode]


The issue was that these functionalities are not available on mobile but I thought they are good additions for desktop size screen too and so made them available for both.

Thanks,

@SebinSong SebinSong self-assigned this Dec 10, 2024
Copy link

cypress bot commented Dec 10, 2024

group-income    Run #3544

Run Properties:  status check passed Passed #3544  •  git commit 0c878daf4b ℹ️: Merge af81aba5cd627a89ccd19f0f1f965022a9f8f848 into 7b1c2b3f5018516781f8955dec2b...
Project group-income
Branch Review sebin/task/#2445-download-and-delete-image-on-mobile
Run status status check passed Passed #3544
Run duration 08m 49s
Commit git commit 0c878daf4b ℹ️: Merge af81aba5cd627a89ccd19f0f1f965022a9f8f848 into 7b1c2b3f5018516781f8955dec2b...
Committer Sebin Song
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 10
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 111
View all changes introduced in this branch ↗︎

Copy link
Member

@taoeffect taoeffect left a comment

Choose a reason for hiding this comment

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

Seems to work really well!

Fantastic work @SebinSong! One minor issue found.

@@ -227,14 +239,18 @@ export default {
openImageViewer (objectURL) {
if (!objectURL) { return }

console.log('!@# attachment list: ', this.attachmentList)
Copy link
Member

Choose a reason for hiding this comment

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

Stray debugging

@SebinSong
Copy link
Collaborator Author

@taoeffect Updated the PR again.

@SebinSong
Copy link
Collaborator Author

@taoeffect as reported in this Slack thread, upload-image functionality is currently broken in master. Pls don't review this PR until this is resolved.

@SebinSong
Copy link
Collaborator Author

@taoeffect Apparently, the broken image upload functionality requires us to refactor the entire image compression logic. Let me make this fix as part of this PR and let you know if it's ready for review again.

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.

Image download/delete options are not available on mobile
2 participants