qtplasmac: fix dual-code button's optional inidicator - #4592
snowgoer540 wants to merge 1 commit into
Conversation
Previously this used "setCheckable", but user button actions are based on "pressed" and "released", and checkable works from "clicked". Since the GUI was also toggling setChecked, it created a race. Allowing qt to determine whether or not it was to be checked or not also did not work reliably. Changed to manually handling the button's sytling which is congruent with what is done elsewhere in the GUI very reliably.
| 'probe-test', 'single-cut', 'torch-pulse', 'user-manual', 'latest-file', 'toggle-joint'] | ||
| head = _translate('HandlerClass', 'User Button Error') | ||
| for bNum in range(1, 21): | ||
| self.w[f'button_{bNum}'].setCheckable(False) |
There was a problem hiding this comment.
This line was also what reset a lit indicator whenever the buttons were rebuilt. What happens if a dual-code button with ;; true is lit when the user saves or reloads the user buttons? user_button_setup() sets the text back to n Name, but what clears the button_active style? And on the next press, which branch runs, and does the indicator still match the text after that?
There was a problem hiding this comment.
You are absolutely correct; thank you for catching that! I added the missing line, all should be well now.
There was a problem hiding this comment.
Thanks, that fixes the reload case. One follow-up: button_normal() now gives all 20 user buttons their own stylesheet, with the colors from that moment. At startup this runs before set_color_styles(), so what color do the user buttons get with a custom stylesheet? And after a color change in Settings, what restyles them? Would self.w[f'button_{bNum}'].setStyleSheet('') reset the indicator just as well, while leaving the colors to the global sheet?
There was a problem hiding this comment.
That is more correct, but it does highlight (pun intended) some other issues. For example, if a button is active when you change the highlight color, it didn't update. It also brings up that if the user changes a user button that was controlling a halpin, and it was active, nothing would stop you. Maybe that's ok maybe it isn't. Sometimes I do wonder how far is appropriate to save the user from themself, and it's definitely an edge case...
At any rate, I have the first issue fixed and ran out of time tonight for the second, but I am not sure if we should commit this as is, and I'll follow up with a commit to address this as well as the other stuff in a bit, or keep force committing to this pr?
There was a problem hiding this comment.
Force push here works, or whatever fits your boat really, we do like to merge somewhat clean history if possible...
fc5cfba to
017b8a4
Compare
2 participants
Previously this used "setCheckable", but user button actions are based on "pressed" and "released", and checkable works from "clicked". Since the GUI was also toggling setChecked, it created a race. Allowing qt to determine whether or not it was to be checked or not also did not work reliably. Changed to manually handling the button's sytling which is congruent with what is done elsewhere in the GUI very reliably.