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

Add New/Preview Entry Attachments dialog and functionality #11637

Draft
wants to merge 4 commits into
base: develop
Choose a base branch
from
Draft

Add New/Preview Entry Attachments dialog and functionality #11637

wants to merge 4 commits into from
+528 −2

Conversation

w15eacre
Copy link
Contributor

@w15eacre w15eacre commented Jan 7, 2025 •
edited
Loading

  • This change adds a new opportunity to add attachments that don’t require a real file in the file system.
  • Add a new dialog window to add attachments and integrate it into the EntryAttachmentsWidget.
  • Add an error message if the file name is invalid and disable the save button.
  • Add the preview button.
  • Add preview with support for: images and text.

Screenshots

empty_name
the same name
ok
widget
preview_webp
preview_gif
preview_mp4
preview_pdf
preview_txt
readonly_preview

Testing strategy

  • Manual testing

Type of change

  • ✅ New feature (change that adds functionality)

Fixes #11506

@w15eacre w15eacre force-pushed the feature/add_attachments_new_button branch from eae01f6 to df95ad6 Compare January 7, 2025 20:37
@droidmonkey
Copy link
Member

droidmonkey commented Jan 7, 2025 •
edited
Loading

Can we pair this with an attachment preview option? Same dialog just in reverse. Can also add an image control on there in case contents are not text.

@w15eacre w15eacre force-pushed the feature/add_attachments_new_button branch from 47d1bb0 to abf2159 Compare January 8, 2025 18:26
@w15eacre
Copy link
Contributor Author

w15eacre commented Jan 8, 2025

@droidmonkey Added text and image preview button.

@w15eacre w15eacre changed the title Add New Entry Attachments dialog and functionality Add New/Preview Entry Attachments dialog and functionality Jan 8, 2025
@droidmonkey
Copy link
Member

Love it!

@w15eacre w15eacre force-pushed the feature/add_attachments_new_button branch from abf2159 to e00aec9 Compare January 8, 2025 23:20
@xboxones1
Copy link
Contributor

xboxones1 commented Jan 9, 2025 •
edited
Loading

Maybe we should remove the "Preview" button and open supported formats by default in the preview window? And for unsupported formats, add a warning that the file will be decrypted and opened in other software. For an example you can see how it is implemented in classic keepass2.

There is also a bug with the preview window style, you can even see it on the screenshots above. If you open it from the entry, everything is fine, if you open it from the "preview panel", it doesn't follow the style.

12

@w15eacre
Copy link
Contributor Author

w15eacre commented Jan 9, 2025 •
edited
Loading

Maybe we should remove the "Preview" button and open supported formats by default in the preview window? And for unsupported formats, add a warning that the file will be decrypted and opened in other software. For an example you can see how it is implemented in classic keepass2.

There is also a bug with the preview window style, you can even see it on the screenshots above. If you open it from the entry, everything is fine, if you open it from the "preview panel", it doesn't follow the style.

12

I will fix this issue with the style. It's strange, because the same window is shown

@w15eacre
Copy link
Contributor Author

w15eacre commented Jan 9, 2025

If we replace the "Open" button, then we should be able to edit the file, but I don't understand how to edit non-text files (PDF, images, ...). I want to support preview PDF, because some services save recovery codes in this format.

@w15eacre
Copy link
Contributor Author

w15eacre commented Jan 9, 2025

@xboxones1 I fixed styles.

I'll check the keepass2 to understand its behavior.

@droidmonkey What do you think about this?

@droidmonkey droidmonkey added user interface ux pr: new feature Pull request that adds a new feature labels Jan 9, 2025
@droidmonkey droidmonkey added this to the v2.7.10 milestone Jan 9, 2025
@droidmonkey droidmonkey self-requested a review January 9, 2025 18:03
Fix read-only preview
3f91ffe 
Refactor EntryAttachmentsWidget and PreviewEntryAttachmentsDialog to remove unnecessary parent references

Fixes keepassxreboot#11506
@w15eacre w15eacre force-pushed the feature/add_attachments_new_button branch from 1c1483a to 3f91ffe Compare January 9, 2025 18:10
@droidmonkey
Copy link
Member

It works great, although for some reason wouldn't let me preview keeagent.settings which is XML (I think, maybe its Base64).

image

I think we can make a couple more enhancements. See above for button adds. We can likely get rid of "Rename" by allowing for direct rename within the list widget.

We should create a little grouping in the button list as well:

image

@w15eacre
Copy link
Contributor Author

w15eacre commented Jan 9, 2025

Added support for xml, json, yaml, soap, protobuf.

Next time I'll start refactoring the UI

@w15eacre w15eacre marked this pull request as draft January 9, 2025 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@droidmonkey droidmonkey Awaiting requested review from droidmonkey

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
pr: new feature Pull request that adds a new feature user interface ux
Projects
None yet
Milestone
v2.7.10
Development

Successfully merging this pull request may close these issues.

Allow creating attachment from textinput (like copy-paste)
3 participants
@w15eacre @droidmonkey @xboxones1