Skip to content

Show empty required object-list fields with warning icon, remove unused IGdefaultRemoved slot#780

Merged
jmarrec merged 1 commit intodevelopfrom
fix_778
Jan 6, 2025
Merged

Show empty required object-list fields with warning icon, remove unused IGdefaultRemoved slot#780
jmarrec merged 1 commit intodevelopfrom
fix_778

Conversation

@macumber
Copy link
Collaborator

@macumber macumber commented Jan 3, 2025

Fixes #778


// field is currently set to an invalid value
idx = 0;
connect(combo, static_cast<void (QComboBox::*)(const QString&)>(&QComboBox::currentTextChanged), this, &InspectorGadget::IGdefaultRemoved);
Copy link
Collaborator Author

@macumber macumber Jan 3, 2025

Choose a reason for hiding this comment

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

The IGdefaultRemoved slot was only connected in this case and it was being called directly from InspectorGadget::layoutComboBox at line 808 combo->setCurrentIndex(idx). This means that the slot was called before the user had a chance to do anything, so it was effectively useless.

Base automatically changed from update-3_9_0 to develop January 6, 2025 11:35
@jmarrec
Copy link
Collaborator

jmarrec commented Jan 6, 2025

@macumber A before/after screenshot or gif would go a long way in helping to see the changes when reviewing.

@jmarrec
Copy link
Collaborator

jmarrec commented Jan 6, 2025

before 4bfb712:

Model with zero thermal zones:

image

One thermal zone: it defaults to "Thermal Zone 1"

image

@jmarrec
Copy link
Collaborator

jmarrec commented Jan 6, 2025

After, this PR

Model with no thermal zone:

image

Model with one TZ, still shows empty, but I can select "Thermal Zone 1"

image

@jmarrec
Copy link
Collaborator

jmarrec commented Jan 6, 2025

I see one (very) minor issue.

I add one or more thermal zones.

Initially it's empty: it shows the warning icon. I can go select "Thermal Zone1". But until I switch tabs, I can reselect the "Empty" slot though.

If I select any zones, switch tabs and go back, then the Empty slot is no longer available.

I'm fine with the current way, but if you know off the top of your head how to prevent re-selecting the empty slot, I'll take it.

@jmarrec jmarrec merged commit 9917e10 into develop Jan 6, 2025
1 check passed
@jmarrec jmarrec deleted the fix_778 branch January 6, 2025 11:57
@github-actions github-actions bot locked and limited conversation to collaborators Jan 6, 2025
@macumber
Copy link
Collaborator Author

macumber commented Jan 7, 2025

I saw the same issue and figured that behavior was probably better than whatever bugs I would introduce trying to clean up the combo box list when the user picked a new option. I heard you on the gifs for PRs, I'll try to be better :-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to assign zone name to Refrigeration:Case

2 participants