Skip to content

feat: support create a desktop for link#89

Open
jeesk wants to merge 2 commits intonwutils:mainfrom
jeesk:main
Open

feat: support create a desktop for link#89
jeesk wants to merge 2 commits intonwutils:mainfrom
jeesk:main

Conversation

@jeesk
Copy link
Copy Markdown

@jeesk jeesk commented Mar 25, 2024

It should be allowed to create a website or deeplink shortcut for Windows, translated into English

Comment thread src/validation.js Outdated
@TheJaredWilcurt
Copy link
Copy Markdown
Member

What do you mean by "deeplink"?

Co-authored-by: The Jared Wilcurt <TheJaredWilcurt@users.noreply.github.com>
@jeesk
Copy link
Copy Markdown
Author

jeesk commented Mar 25, 2024

What do you mean by "deeplink"?

Custom protocols, such as Lazycat, can be used for Deeplink links lazycat://index/1?key1=value1&key2=value2. In general, Deeplink is a protocol registered by the application itself, and the application can be opened through Deeplink.

Our Electron application has registered some of our own Deeplinks, please refer to the following address: https://www.electronjs.org/docs/latest/tutorial/launch-app-from-url-in-another-app

Comment thread src/validation.js
Comment on lines 398 to 399
windowsFilePath = helpers.resolveWindowsEnvironmentVariables(windowsFilePath);
windowsFilePath = this.resolvePATH(windowsFilePath);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  • There needs to be some basic safety checking for this feature (see suggestion below).
  • Also the isLink should be checked as a optional boolean defaulting to false in the validation file.
  • There needs to be unit tests created to ensure the repo stays at 100% test coverage.
  • The documentation in the README needs updated as well to convey this new feature.
Suggested change
windowsFilePath = helpers.resolveWindowsEnvironmentVariables(windowsFilePath);
windowsFilePath = this.resolvePATH(windowsFilePath);
if (options.windows.isLink) {
if (
typeof(windowsFilePath) !== 'string' ||
!windowsFilePath.includes('://')
) {
helpers.throwError(options, 'WINDOWS filePath must be a string of a URL when isLink is true:' + windowsFilePath);
delete options.windows;
}
return options;
}
windowsFilePath = helpers.resolveWindowsEnvironmentVariables(windowsFilePath);
windowsFilePath = this.resolvePATH(windowsFilePath);

@TheJaredWilcurt
Copy link
Copy Markdown
Member

Custom protocols on desktop apps aren't a new concept, just didn't know that Electron is giving them the name "deep links", which I've only heard in reference to Mobile apps, but the concept is the same. I wish there was a generic Node module to register custom protocols on each platform.

Anyways. I added some comments as to the rest of the work needed before this PR can be merged. If you prefer not to do it, you can create an issue instead and I'll get to it at some point.

@jeesk
Copy link
Copy Markdown
Author

jeesk commented Mar 26, 2024

Custom protocols on desktop apps aren't a new concept, just didn't know that Electron is giving them the name "deep links", which I've only heard in reference to Mobile apps, but the concept is the same. I wish there was a generic Node module to register custom protocols on each platform.

Anyways. I added some comments as to the rest of the work needed before this PR can be merged. If you prefer not to do it, you can create an issue instead and I'll get to it at some point.

good job, thanks.

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.

2 participants