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

CID: Disable btnDefault if no Custom Install Directory path is set #457

Conversation

sonic2kk
Copy link
Contributor

Based on suggestion in #454 (comment).

This PR disables the "Default" button (btnDefault) if no Custom Install Directory has been configured. It also disables the button on click, if it successfully clears the value from the INI (i.e. if after clicking, the Custom Install Directory is successfully removed, toggle btnDefault enabled/disabled based on the bool cast of the value in the INI file).

The Custom Install Directory path is saved to the ProtonUp-Qt config file, and we can retrieve it using config_custom_install_location. I believe this is also responsible for writing out the path as well to the INI file. We can toggle whether btnDefault is enable on dialog load based on if custom_install_directory is defined.

custom_install_directory: str = config_custom_install_location().get('install_dir', '') will retrieve the value of install_dir based on the value in the INI. If this is not set, trying to fetch install_dir will actually return None. I am not entirely sure why this is, I would've thought calling .get('install_dir', '') would return '' for Falsey values, but I guess not. The config_custom_install_location function can take an install_dir keyword argument and this defaults to install_dir=None, so that's where the None comes from. If no Custom Install Directory is set, config_custom_install_location returns None.

Instead of refactoring behaviour in config_custom_install_location for this PR and potentially breaking things (i.e. changing the default argument to install_dir=''), I rolled with it and called setEnabled with a bool cast of custom_install_directory. If custom_install_directory returns None (which it should if there is no Custom Install Directory is set) then we call bool(None) to toggle the enabled state of btnDefault, which will disable it (bool(None) is False). Also, if custom_install_directory contained other Falsey values like an empty string ('') (which shouldn't be possible normally, but could happen if a user manually edited their INI file), we would also disable btnDefault. I think this is desirable; Falsey values should be considered equal to having no Custom Install Directory set, and so should disable btnDefault

Since this functionality is also used on btnDefault click, I broke it out into a method that handles checking if a custom_install_directory is set to a Truthy value. This makes the code a bit cleaner, as instead of doing the bool cast ourselves, we just tell btnDefault.setEnabled to toggle based on self.has_custom_install_directory return value, and that method handles the implementation detail. This also means if we wanted to change this implementation in future it would be more straightforward (e.g. if we ever wanted to validate the path in any way).

I have attached some screenshots to show this change.

Before (main)

image

After (This PR)

image

It is also enabled again when a Custom Install Directory is saved and the dialog is re-opened.

image


Thanks!

@DavidoTek
Copy link
Owner

Thanks!

@DavidoTek DavidoTek merged commit 01dd509 into DavidoTek:main Sep 20, 2024
1 check passed
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