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: Connect txtInstallDirectory.textChanged sooner #456

Merged
merged 2 commits into from
Sep 20, 2024

Conversation

sonic2kk
Copy link
Contributor

Fixes an issue I found when investigating another issue relating to the Custom Install Directory dialog.

Overview

This PR connects txtInstallDirectory.textChanged sooner, so that it can get triggered when we call txtInstallDirectory.setText. textChanged does validate text that is set programmatically, but won't do anything for our case if we connect it after we already set the text. This fixes an issue where when opening the Custom Install Directory dialog with a selected Custom Install Directory, the pre-populated path in the txtInstallDirectory Line Edit would not be seen as valid (because the textChanged signal to validate the path was not connected until after the text was set, so it did not know to validate the path on open of the dialog) and so the "Save" button would be kept as disabled.

Background

Currently, when you enter a path manually into the Custom Install Directory's txtInstallDirectory Line Edit, it will validate the path and dynamically enable/disable the "Save" button based on whether the path is valid (exists and that we have write access to the folder). We use textChanged because this value can also be updated programmatically based on the File Picker that can appear as well. By default, this button is disabled, because by default the txtInstallDirectory Line Edit is blank, so we don't want "Save" to be enabled.

However, if a Custom Install Directory is already saved, when opening the Custom Install Directory dialog, the path is populated, but the "Save" button is kept as disabled. This is because we connect the txtInstallDirectory.textChanged signal after we call txtInstallDirectory.setText. The order is like this:

  • btnSave is disabled by default
  • txtInstallDirectory text is populated to the current Custom Install Directory path
  • txtInstallDirectory.textChanged is connected to validate the text in the field, but will not validate any path entered before this connection.
  • Pre-populated text is not validated when it should be, so even if the path is valid, btnSave is kept as disabled until the text in the txtInstallDirectory dialog is updated again (i.e. removing the trailing forward-slash).

Solution

The solution here is to connect txtInstallDirectory.textChanged before calling txtInstallDirectory.setText, which allows calls to setText to be validated by the textChanged action.

The effect of this results in the following behaviour:

Before (main)

image

After (This PR)

image


Likely there is no reason why a user would want to save the path again to the pre-populated path, but I don't think the "Save" button needs to care about that. When the path is valid, even if it is a pre-populated path, the "Save" button should not be disabled. If I found this confusing behaviour I am sure others have too. 😄

Thanks!

@DavidoTek
Copy link
Owner

Thanks!

However, if a Custom Install Directory is already saved, when opening the Custom Install Directory dialog, the path is populated, but the "Save" button is kept as disabled.
the "Save" button should not be disabled. If I found this confusing behaviour I am sure others have too. 😄

I guess there is not much reason to "save" it if there have been no changes, but okay, I understand that it can be a bit confusing.

@DavidoTek DavidoTek merged commit d7c71a8 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