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

Get the desktop folder from the system instead of hardcoding '/Desktop' #722

Merged

Conversation

arielj
Copy link
Collaborator

@arielj arielj commented Oct 31, 2021

Use xdg-user-dir DESKTOP on linux systems to get the correct Desktop folder path that depends on the system language.

My system is in spanish, so my Desktop folder is actually Escritorio, using the xdg-user-dir command returns the full path to that directory.

This is only implemented for linux since adding shortcuts to desktop is only implemented on linux.

Closes #721

I couldn't add a test since I would have to stub the system command somehow which feels too hacky, but I accept any suggestion on how to add a test for this fix!


Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

@arielj arielj changed the title Get the desktop folder from the system instead of harcoding '/Desktop' Get the desktop folder from the system instead of hardcoding '/Desktop' Oct 31, 2021
@@ -15,10 +16,24 @@ function getLegendaryBin(){
return bin
}

// Get the "Desktop" directory with multilingual support, fallback to "~/Desktop"
function getDesktopPath() {
Copy link
Member

Choose a reason for hiding this comment

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

Instead of using this you can simply get the desktop folder using:
app.getPath('desktop') importing app from electron.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oh, first time ever using electron, I didn't know this, thanks! I'll update the PR

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I couldn't use app.getPath('desktop') here in the constants.ts file because electron's app is undefined during tests and it makes the tests fail, so I used that directly on the games.ts file

Copy link
Member

@flavioislima flavioislima left a comment

Choose a reason for hiding this comment

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

Looks good but use the builtin electron function to the get desktop folder since it will work for other platforms as well.

Copy link
Member

@flavioislima flavioislima left a comment

Choose a reason for hiding this comment

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

Looks good 👍🏽

@flavioislima flavioislima merged commit bba8226 into Heroic-Games-Launcher:main Oct 31, 2021
@arielj arielj deleted the multilingual-desktop-folder branch November 6, 2021 01:33
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.

Adding a game shortcut only creates the Start Menu shortcut but not the desktop shortcut on non-english system
2 participants