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

Better handle disabling widgets when displaying #308

Merged
merged 2 commits into from
May 1, 2024

Conversation

mwcraig
Copy link
Contributor

@mwcraig mwcraig commented Apr 29, 2024

This addresses the first two items in #303, so when merged it fixes #303.

The original method of disabling and re-enabling the fields of a model when displaying vs editing or making a new item was a little too aggressive, and ended up enabling something elements that should not have been enabled.

This fixes that by only disabling AutoObjects and also hiding the add/remove buttons that appear in the PassbandMap object.

@codecov-commenter
Copy link

codecov-commenter commented Apr 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.76%. Comparing base (ae22f87) to head (66e376d).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #308      +/-   ##
==========================================
+ Coverage   73.71%   73.76%   +0.04%     
==========================================
  Files          27       27              
  Lines        3390     3396       +6     
==========================================
+ Hits         2499     2505       +6     
  Misses        891      891              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mwcraig mwcraig force-pushed the fix-autoui-disabled-state branch from cf1804c to 66e376d Compare April 30, 2024 13:35
top.disabled = value
elif isinstance(top, ItemBox):
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll admit I am trusting your tests because I don't know pyautoui well.

# There is no great way to get to the ItemBox widget that contains and controls
# the add/remove buttons, so we keep going down through widget children until we
# get to an ItemBox and then check that the buttons are disabled.
# Recursion is the easiest way to do that, so recurse we will..
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, an actual use for recursion that isn't factorials or the Fibonacci sequence or binary search...

Copy link
Contributor

@JuanCab JuanCab left a comment

Choose a reason for hiding this comment

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

Looks good as tests go. I am not an pyautoui expert, so I am relying on the tests as described working as commented.

@Tanner728
Copy link
Contributor

Doing some more testing here and noticed some odd interaction.
When creating a new passband map for the first time you are able to click the Edit this Passband Map button without having entered a passband name.

This results in you not being able to enter a name for the passband map and therefore unable to save it. Is this something we need to address? Beyond this I did not find anymore bugs... yet.

image

@mwcraig
Copy link
Contributor Author

mwcraig commented May 1, 2024

When creating a new passband map for the first time you are able to click the Edit this Passband Map button without having entered a passband name.

Good catch -- I think that was fixed by #307 but will check once all of these are merged

@mwcraig mwcraig merged commit 967dc27 into feder-observatory:main May 1, 2024
9 checks passed
@mwcraig mwcraig deleted the fix-autoui-disabled-state branch May 1, 2024 13:28
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.

Fix some issues with the custom widget UI
4 participants