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

feat/add context menu #236

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

feat/add context menu #236

wants to merge 11 commits into from
+639 −223

Conversation

xbozo
Copy link
Contributor

@xbozo xbozo commented May 8, 2024 •
edited

image

  1. Open archive location --> if user has already selected an exe it opens the folder based on this exe location. If not, it opens the installation folder path.
  2. Change executable path --> Self-explained.
  3. Remove installation archives --> Deletes installation folder; the one located on the download path. Keeps other files (such as the folder where the selected exe with the installed game is) intact. Helpful to get rid of unused archives and free disk space. (includes dialog to confirm if user wants to proceed with action).

solve #220
solve #130
solve #118
solve #71
solve #416
solve #484

partial solve #421
partial solve #468

lilezek
lilezek suggested changes May 9, 2024
View reviewed changes
src/main/events/library/open-game-folder.ts
Comment on lines +18 to +24
if (game.executablePath) {
gamePath = path.dirname(game.executablePath);
} else {
gamePath = game.folderName
? path.join(await getDownloadsPath(), game.folderName)
: await getDownloadsPath();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This opens either the installer folder or the game folder, but I think this may be confusing. The installer's folder and the executable's path should be something different. We could disable the option in the context menu if the path is missing.

Copy link
Contributor Author

@xbozo xbozo May 9, 2024 •
edited

Choose a reason for hiding this comment

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

Since games can have a "done installation" folder (where the .exe is located at) and a "to install" folder (where the default installation path is set at configs) it would be a better experience to always have something addressed to "Open archive location" button, if there is any archives at all.

src/renderer/src/components/sidebar/sidebar.tsx Outdated Show resolved Hide resolved
src/renderer/src/components/sidebar/sidebar.tsx Outdated Show resolved Hide resolved
src/renderer/src/components/sidebar/sidebar.tsx Outdated Show resolved Hide resolved
src/renderer/src/hooks/use-download.ts Outdated Show resolved Hide resolved
src/renderer/src/pages/game-details/hero/hero-panel-actions.tsx Outdated Show resolved Hide resolved
@xbozo
Copy link
Contributor Author

xbozo commented May 9, 2024

adjusted

@lilezek
Copy link
Contributor

lilezek commented May 9, 2024

adjusted

Everything looks good but the transformation from <ul> to <div>. Can you leave it to be an <ul> as it was originally?

Also, I'm still wondering why is updating the library needed before opening a game. I'm not saying it is wrong, but I don't get it. Could you elaborate?

@xbozo
Copy link
Contributor Author

xbozo commented May 9, 2024

adjusted

Everything looks good but the transformation from <ul> to <div>. Can you leave it to be an <ul> as it was originally?

Also, I'm still wondering why is updating the library needed before opening a game. I'm not saying it is wrong, but I don't get it. Could you elaborate?

debugging reasons.
changed already

@lilezek
Copy link
Contributor

lilezek commented May 9, 2024

LGTM

@xbozo
Copy link
Contributor Author

xbozo commented May 23, 2024 •
edited

review/merge please
@thegrannychaseroperation
@zamitto

zamitto
zamitto reviewed May 27, 2024
View reviewed changes
src/main/entity/game.entity.ts Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need to have changes on this file. Keep as little changes as possible, please

zamitto
zamitto reviewed May 27, 2024
View reviewed changes
src/main/events/misc/show-open-dialog.ts Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need to have changes on this file. Keep as little changes as possible, please.

And AFAIK, the function used in the event needs to be asyn

zamitto
zamitto reviewed May 27, 2024
View reviewed changes
src/main/events/library/open-game.ts Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need to have changes on this file. Keep as little changes as possible, please

zamitto
zamitto reviewed May 27, 2024
View reviewed changes
src/renderer/src/assets/discord-icon.svg Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

Discord icon is not used anymore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should sub for telegram?

zamitto
zamitto reviewed May 27, 2024
View reviewed changes
src/main/events/library/delete-game-folder.ts Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need to have changes on this file. Keep as little changes as possible, please.

@zamitto
Copy link
Collaborator

zamitto commented May 27, 2024

Are you using Prettier with the project settings? A few changes seems that would be fixed by prettier. And overall, try to make as little changes as possible. I made just a quick review

@xbozo
Copy link
Contributor Author

xbozo commented May 28, 2024 •
edited

Are you using Prettier with the project settings? A few changes seems that would be fixed by prettier. And overall, try to make as little changes as possible. I made just a quick review

I am using the prettier.

@xbozo
Copy link
Contributor Author

xbozo commented May 28, 2024

adjusted

@xbozo xbozo requested a review from zamitto May 28, 2024 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lilezek lilezek lilezek requested changes

@zamitto zamitto Awaiting requested review from zamitto

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

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

Successfully merging this pull request may close these issues.

None yet
3 participants
@xbozo @lilezek @zamitto