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

Added a menu-item to show Folder and Requests in native file-browser #3787

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ramki-bruno
Copy link
Collaborator

@ramki-bruno ramki-bruno commented Jan 13, 2025

Duplicate of #3698
Closes #3541

Description

On clicking the triple-dot menu(kebab menu?) on a Folder or a Request, a new menu-item named Show in Folder is added which opens it in the native file-browser(Finder, File-Explorer, Nautilus, Ranger, etc.)

image
image

* feat: Reveal in Finder

* added support for linux
shell.showItemInFolder(resolvedPath);
break;
case 'linux': // Linux
exec(`xdg-open "${resolvedPath}"`);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Electron internally seems to be using xdg-open anyways for opening files in Linux. I don't think these platform-specific switch-cases are necessary.
https://github.com/electron/electron/blob/v33.2.1/shell/common/platform_util_linux.cc#L342-L347

@NV404 I'm replacing this with just one line shell.showItemInFolder(...). Let me know if you think otherwise.

throw new Error('File path is required.');
}

const resolvedPath = path.resolve(filePath);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

path.resolve here will not do anything useful since it works with the app process's CWD(probably /) and resolving the given path against it doesn't make much sense


const resolvedPath = path.resolve(filePath);

if (!fs.existsSync(resolvedPath)) {
Copy link
Collaborator Author

@ramki-bruno ramki-bruno Jan 13, 2025

Choose a reason for hiding this comment

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

Probably not worth the extra delay since when this IPC address if used right, this case will rarely happen.

@@ -227,6 +227,12 @@ const CollectionItem = ({ item, collection, searchText }) => {
}
};

const handleShowInFolder = () => {
dispatch(showInFolder(item.pathname)).catch((error) => {
toast.error('Error opening the folder:', error);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

toast.error doesn't take the 2nd arg(probably confused with console.error I assume)
https://react-hot-toast.com/docs/toast#error

@ramki-bruno ramki-bruno force-pushed the feature/reveal-in-finder branch from f3b2a59 to 7613777 Compare January 13, 2025 22:15
@ramki-bruno ramki-bruno force-pushed the feature/reveal-in-finder branch from 7613777 to 29f51aa Compare January 13, 2025 22:42
@ramki-bruno
Copy link
Collaborator Author

Tested on Mac. Need to test on Linux and Windows
Other than that, LGTM.

@ramki-bruno ramki-bruno changed the title Added a menu-item to show Folder and Requests within Collection in native file-browser Added a menu-item to show Folder and Requests in native file-browser Jan 13, 2025
@ramki-bruno
Copy link
Collaborator Author

Prefer merge-commit, no Squash and merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Reveal in Finder/Explorer when right-clicking request/folder
2 participants