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

Propagate "update check" prompt to UI checkbox #515

Merged
merged 3 commits into from
Aug 23, 2023

Conversation

deeplow
Copy link
Contributor

@deeplow deeplow commented Aug 14, 2023

The "check for updates" button wasn't showing up immediately as checked as soon as the user is prompted for checking updates. This fixes that.

Fixes #513

@deeplow
Copy link
Contributor Author

deeplow commented Aug 14, 2023

@apyrgio I was not sure what the best state propagation method was. I opted for sending a "refresh" signal, but the state is written to and read from the settings.

We could do that through the signal and it would also be fine, especially since this is an edge case and we don't need to keep track of that propagated state (like the docs_added we had in the main_window a while ago).

tests/gui/test_updater.py Outdated Show resolved Hide resolved
dangerzone/gui/__init__.py Outdated Show resolved Hide resolved
@deeplow
Copy link
Contributor Author

deeplow commented Aug 21, 2023

@apyrgio that's way more elegant. Let's go with that. I haven't made a test, though since I didn't find a decent place to go a test for this. Since the behavior only really happens in gui/__init__.py. Do you have suggestions regarding testing this? Or should we leave it untested?

@apyrgio
Copy link
Contributor

apyrgio commented Aug 22, 2023

Hm, you're right, there's no easy way to test it. Given that it's a one line thing, maybe we can just test it manually.

@deeplow
Copy link
Contributor Author

deeplow commented Aug 22, 2023

Anything else missing @apyrgio?

@apyrgio
Copy link
Contributor

apyrgio commented Aug 22, 2023

Nope, just checked it manually as well, and it seems to work. Feel free to merge it with just the latest commit.

The "check for updates" button wasn't showing up immediately as checked
as soon as the user is prompted for checking updates. This fixes that.

Fixes #513
Ensure the status of the toggle updates checkbox is updated, after the user is
prompted to enable updates.
@deeplow deeplow merged commit 8ae88eb into main Aug 23, 2023
23 checks passed
@apyrgio apyrgio deleted the 513-update-check-propagate branch October 2, 2023 17:32
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.

Update check does not place "Check for updates" checkmark in UI
2 participants