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

Open zim location in file manager #970

Merged
merged 4 commits into from
Aug 4, 2023
Merged

Open zim location in file manager #970

merged 4 commits into from
Aug 4, 2023

Conversation

juuz0
Copy link
Collaborator

@juuz0 juuz0 commented Aug 2, 2023 •
edited
Loading

Fixes #98
Fixes #971

@kelson42
Copy link
Collaborator

kelson42 commented Aug 2, 2023

@juuz0 Thx, will review later today!

mgautierfr
mgautierfr requested changes Aug 2, 2023
View reviewed changes
Copy link
Member

@mgautierfr mgautierfr left a comment

Choose a reason for hiding this comment

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

I have not tested the functional part.
On the code part, only a small code change and we are good.

src/contentmanager.cpp Outdated Show resolved Hide resolved
@juuz0
Show correct context menu options in All files view
2eb90bd 
Earlier, context menu showed "Open"/"Delete" options only in local library.
This doesn't make sense when browsing the remote library - already downloaded files should show "Open"/"Delete" instead of "Download book"
@juuz0
Add translations for context menu
08b2f25 
Added translations for context menu options:
Delete book
Download book
Open Book
Resume Download
Pause Download
Cancel download
@juuz0 juuz0 requested a review from mgautierfr August 2, 2023 16:14
@kelson42
Copy link
Collaborator

kelson42 commented Aug 2, 2023 •
edited
Loading

I will check and merge, but clearly the edge case to test is the handling of exception if the folder is not redable or not available.

kelson42
kelson42 requested changes Aug 2, 2023
View reviewed changes
Copy link
Collaborator

@kelson42 kelson42 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, but we should have a proper error message if not possible to open a folder. Something like "Kiwix is not able to open folder xyz`.

@juuz0
Copy link
Collaborator Author

juuz0 commented Aug 3, 2023

Something like "Kiwix is not able to open folder xyz`.

Done. Added checks if directory exists/isReadable

@juuz0 juuz0 requested a review from kelson42 August 3, 2023 04:44
@juuz0 juuz0 mentioned this pull request Aug 3, 2023
Closed
@juuz0
Add "Open Folder" context menu option
274aa4d 
Added an option to open the zim's location in the OS's file manager
@juuz0
Do index check before access in contextmenu
63b9de9 
Fix #971
@kelson42
Copy link
Collaborator

kelson42 commented Aug 3, 2023

@mgautierfr Good for you or still have concerns regarding current implementation/solution?

@mgautierfr mgautierfr merged commit 8fa0634 into main Aug 4, 2023
4 checks passed
@mgautierfr mgautierfr deleted the openFolder branch August 4, 2023 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mgautierfr mgautierfr mgautierfr approved these changes

@kelson42 kelson42 kelson42 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

App crash on mouse right click in local library Add "Open folder" command to right-click menu
3 participants
@juuz0 @kelson42 @mgautierfr