[feat-560] ask user for installation folder/path #566

silentrald · 2024-08-23T16:57:16Z

Scope

Implementation

Added a way to ask the user for an existing or non-existing installation folder/path

Modified modal.component.tsx to add the property closable. If closable = false, then you can't exit out the modal by any means, except by adding a button to resolve the modal itself.

Created AskInstallPathModal (ask-install-path-modal.component.tsx).

Created SetupService (setup.service.ts) class to handle any user configuration before application startup. Logic such as sequential modals/prompts can be added here sequentially to ask for config values. Not sure if one whole modal or separate modals will be done but should be easy enough to edit the logic there.

Created bs-installer-ipcs.ts to handle all the server calls for the installation folder/path. Transferred some codes from bs-download-ipcs.ts to here.

Might need to transfer the the installation folder/path code from SteamDownloaderService to another class as well, but might do it in another PR.

Screenshots

UI may still change depending on suggestions
As of commit 9e2f716

How to Test

Test Case

Edit the installation-folder property in the config.json file with any existing or non-existing folder path other than your original installation folder.

Windows: %AppData%/Roaming/bs-manager
Linux: /.config/bs-manager

Open BSManager and a modal will appear (similar to the above screenshot)
Select any folder and click on Confirm
On close either:
4.1. If existing installation folder: version should be shown on the sidepanel
4.2. If non-existing folder: installation folder will be created

Additional Tests

Check if the Default button works properly.
Check if this would not ask the user if the installation path points to an existing installation folder.
Check if the songDetailsCache was loaded properly.

New/Updated Resource Text

misc.confirm: Confirm
pages.settings.installation-folder.description: Change the folder that will contain all the content downloaded by BSManager.
modals.ask-install-path.title: Installation folder
modals.ask-install-path.choose-folder-description: Choose the folder that will contain all the content downloaded by BSManager.
modals.ask-install-path.choose-folder: Choose folder
modals.ask-install-path.default: Default
modals.ask-install-path.default-tooltip: Defaults to your home folder

NOTE: ticked are already translated to other languages.

Emoji Guide

For reviewers: Emojis can be added to comments to call out blocking versus non-blocking feedback.

E.g: Praise, minor suggestions, or clarifying questions that don’t block merging the PR.

🟢 Nice refactor!

🟡 Why was the default value removed?

E.g: Blocking feedback must be addressed before merging.

🔴 This change will break something important


Blocking	🔴 ❌ 🚨	RED
Non-blocking	🟡 💡 🤔 💭	Yellow, thinking, etc
Praise	🟢 💚 😍 👍 🙌	Green, hearts, positive emojis, etc

Zagrios · 2024-08-24T14:00:34Z

@silentrald Maybe its a good its to add an explaining text between the title and the "choose folder" field 🤔
Something like "Choose the folder that will contain all the content downloaded by BSMananger" (its juste an exemple but yeah something like that)

silentrald · 2024-08-24T15:07:08Z

@Zagrios that should be fine, will follow "Choose the folder that will contain all the content downloaded by BSManager".

Might need to update the settings page explanation text as well if that's the case, to pattern these two. Will still use the above text and change "Choose" to "Change". Although, what do you think, should I change it or leave it as is?

Also for the translation, should I ping someone in the discord channel so that I can fit it in the PR, or do we delay the translation and let someone just translate it before the next alpha release?

Zagrios · 2024-08-24T19:32:42Z

@silentrald you can change it, it's sounds good to me 👍

I also notice that you made change to the cache system to wait for the user choosing a folder, thats made me wondering if I should move those cache files to the installation folder of BSM that's will always be on the "main" drive of the user 🤔 Cause all the cache files are there to improve perfomances, and the user's "main" drive is more likely to be an hight performance drive (like ssd rather than a hdd or a local network drive) 🤔

Zagrios · 2024-08-24T19:44:03Z

@silentrald Also maybe add a Tooltip on on the "Default" button to indicate that this will chose the home folder of the user
(you can find a lot of exemples searching for Tippy in the code; You will probably need to add a div wrapper for the button but I'll let you see by yourself)

silentrald · 2024-08-25T01:33:21Z

silentrald you can change it, it's sounds good to me 👍

I also notice that you made change to the cache system to wait for the user choosing a folder, thats made me wondering if I should move those cache files to the installation folder of BSM that's will always be on the "main" drive of the user 🤔 Cause all the cache files are there to improve perfomances, and the user's "main" drive is more likely to be an hight performance drive (like ssd rather than a hdd or a local network drive) 🤔

Yeah I think moving it to the installation folder might be better. Something like a .cache or Cache folder might suffice but might need be handled in another PR since this is out of scope of the current feature.

Also context for why I did that as well, had to delay the instantiation of the ElectronStore object since this automatically creates the installation folder and the config.cfg file, which makes the modal asking for the user for an installation folder not show up. This makes the configuration service not yet readable/writable until setup is done, delaying the loading the cache files. Although this doesn't feel like a bug since there is a chance to read the config file on the installation folder that the user selected.

Zagrios · 2024-08-25T11:04:58Z

src/shared/models/ipc/ipc-routes.ts

@@ -43,6 +41,12 @@ export interface IpcChannelMapping {
    "bsv-search-playlist": {request: PlaylistSearchParams, response: BsvPlaylist[]};
    "bsv-get-playlist-details-by-id": {request: {id: string, page: number}, response: BsvPlaylistPage};

+    /* ** bs-installer-ipcs ** */
+    "bs-installer.folder-exists": { require: void, response: boolean };


You wrote require here

Suggested change

"bs-installer.folder-exists": { require: void, response: boolean };

"bs-installer.folder-exists": { request: void, response: boolean };

Zagrios · 2024-08-25T11:12:02Z

src/renderer/components/shared/bsm-button.component.tsx

+        return (
+            <Tippy
+                className="!bg-main-color-1"
+                content={t(tooltip)}


I meant, adding a tooltip while hovering the button globally, like this users are sure to see the tooltip.
So no need to add all this stuff in the button component

Ahhh, sorry about that, thought we needed an icon for it, will apply a commit for this.

src/main/ipcs/setup.ipcs.ts

Zagrios · 2024-08-25T11:20:42Z

src/renderer/services/setup.service.ts

+            }
+
+            let modalResponse: ModalResponse<{ installPath: string }> = { exitCode: ModalExitCode.NO_CHOICE };
+            while (modalResponse.exitCode !== ModalExitCode.COMPLETED) {


Do we still need the while loop here ? since the modale is marked as non closable ?

Yeah, I think the loop there can be removed as well

Zagrios · 2024-09-03T18:13:33Z

@silentrald can you resove confict ?
No need for the cache to wait the setup, we have now a "staticConfig"

added check if ElectronStore is null

added tooltip support in BsmButton component

silentrald · 2024-09-04T01:04:10Z

@Zagrios rebased and resolved the conflicts. Also removed the waiting on setup as well.

Zagrios · 2024-09-04T18:55:14Z

@silentrald I've found a little bug, when I select a folder from the modal, the installation path doesn't change in the settings, and I have this error:

Error: ENOENT: no such file or directory, scandir 'D:\\BSManager' at async SetupService.check (http://localhost:1212/renderer.dev.js:248304:13)

See gif:

silentrald · 2024-09-05T01:41:50Z

@Zagrios yep, seems that I haven't covered the case where the oldDir doesn't exist in the InstallationLocationService.setInstallationDirectory. Tested in my windows VM, can set the new folder without moving a non-existent one.

sonarqubecloud · 2024-09-10T19:19:37Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

Zagrios

That seems good to me 🙌
Thanks !

silentrald force-pushed the feat/560 branch from 7bf348a to d2abd4e Compare August 23, 2024 17:03

Zagrios requested changes Aug 25, 2024

View reviewed changes

silentrald added 5 commits September 4, 2024 08:21

[feat-560] ask user for installation folder/path

5e6837b

[feat-560] delay song details cache loading after user setup is done

d3c7970

added check if ElectronStore is null

[feat-560] added description text and default button tooltip

6258de5

added tooltip support in BsmButton component

[feat-560] applied PR comments

9c86adf

[refactor] created installation-location.service.ts on client side

9734d71

silentrald force-pushed the feat/560 branch from 41ad783 to 9734d71 Compare September 4, 2024 00:57

[feature-560] some adjustments

7ae097b

[bugfix-560] don't move old installation path if it doesn't exist

bd371b5

[feature-560] translations

e14ba27

Zagrios linked an issue Sep 10, 2024 that may be closed by this pull request

[FEAT.] : Ask users to select their installation folder on startup #560

Closed

Zagrios linked an issue Sep 10, 2024 that may be closed by this pull request

[BUG] : Not possible to change Installation Folder #481

Closed

Zagrios approved these changes Sep 10, 2024

View reviewed changes

Zagrios merged commit fd52fb8 into Zagrios:master Sep 11, 2024
5 checks passed

silentrald mentioned this pull request Sep 15, 2024

[FEAT.] : make linux AppImage work out of the box #578

Closed

silentrald deleted the feat/560 branch September 15, 2024 09:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feat-560] ask user for installation folder/path #566

[feat-560] ask user for installation folder/path #566

silentrald commented Aug 23, 2024 •

edited by Zagrios

Loading

Zagrios commented Aug 24, 2024

silentrald commented Aug 24, 2024 •

edited

Loading

Zagrios commented Aug 24, 2024

Zagrios commented Aug 24, 2024

silentrald commented Aug 25, 2024

Zagrios Aug 25, 2024

Zagrios Aug 25, 2024

silentrald Aug 25, 2024

Zagrios Aug 25, 2024

silentrald Aug 25, 2024

Zagrios commented Sep 3, 2024

silentrald commented Sep 4, 2024

Zagrios commented Sep 4, 2024

silentrald commented Sep 5, 2024

sonarqubecloud bot commented Sep 10, 2024

Zagrios left a comment

	"bs-installer.folder-exists": { require: void, response: boolean };
	"bs-installer.folder-exists": { request: void, response: boolean };

[feat-560] ask user for installation folder/path #566

[feat-560] ask user for installation folder/path #566

Conversation

silentrald commented Aug 23, 2024 • edited by Zagrios Loading

Scope

Implementation

Screenshots

How to Test

Test Case

Additional Tests

New/Updated Resource Text

Emoji Guide

Zagrios commented Aug 24, 2024

silentrald commented Aug 24, 2024 • edited Loading

Zagrios commented Aug 24, 2024

Zagrios commented Aug 24, 2024

silentrald commented Aug 25, 2024

Zagrios Aug 25, 2024

Choose a reason for hiding this comment

Zagrios Aug 25, 2024

Choose a reason for hiding this comment

silentrald Aug 25, 2024

Choose a reason for hiding this comment

Zagrios Aug 25, 2024

Choose a reason for hiding this comment

silentrald Aug 25, 2024

Choose a reason for hiding this comment

Zagrios commented Sep 3, 2024

silentrald commented Sep 4, 2024

Zagrios commented Sep 4, 2024

silentrald commented Sep 5, 2024

sonarqubecloud bot commented Sep 10, 2024

Quality Gate passed

Zagrios left a comment

Choose a reason for hiding this comment

silentrald commented Aug 23, 2024 •

edited by Zagrios

Loading

silentrald commented Aug 24, 2024 •

edited

Loading