SHOT-2850 Small update to the task_required UI display #130
Conversation
Codecov Report
@@ Coverage Diff @@
## master #130 +/- ##
==========================================
+ Coverage 71.99% 72.89% +0.90%
==========================================
Files 13 13
Lines 1296 1306 +10
==========================================
+ Hits 933 952 +19
+ Misses 363 354 -9
Continue to review full report at Codecov.
|
python/tk_multi_publish2/dialog.py
Outdated
| self.ui.context_widget.ui.task_label.setStyleSheet("") | ||
| self.ui.context_widget.ui.label.setText( | ||
| "Task and Entity Link to apply to the selected item:" |
There was a problem hiding this comment.
Perhaps we should be calling the enable_editing method, and passing the message rather than directly modifying the label. Or using the context_label. I don't think the ui property is intended to be public, despite it not having _ at the beginning.
This also applies further up on line 1665.
Also I assume this works when you select an item, but what happens when you select the summary (visible when you have more than 2 root items), the message should be different I think?
There was a problem hiding this comment.
I'd like to add another comment to Phil's :)
When you have more than one item in your publisher and if some of your items has been setup correclty (aka are linked to a Task), the "error" message still appears for these items. Maybe we should update the message according to the selected item.
Working for single file so committing now.
Style changes apply to each item and to the summary.
|
Alright, I have updated this based on your feedback @pscadding and @barbara-darkshot . |
|
Regarding the Publish/Validate stylesheet, if using the SG_ALERT_COLOR is too flashy, maybe we can just add or modify the tooltip when overflying the buttons saying why they are disabled |
There was a problem hiding this comment.
Thanks for the changes @rob-aitchison, I have a few more suggestions.
| "<p>Task Required is <b>True</b> in your configuration. " | ||
| "Please select a Task to continue.</p>", | ||
| ) | ||
| self.ui.context_widget.ui.label.setStyleSheet( |
There was a problem hiding this comment.
I think you should be able to use context_widget.context_label instead of going via the ui attribute?
I'm uncomfortable with us using the .ui property as that is not intended to be public .
There was a problem hiding this comment.
Also could the color be set through the enable_editing, via html tags? That would save you having to modify the stylesheet.
| # Check for task_required and change UI styling accordingly | ||
| if not self._validate_task_required(): | ||
| # Change task label color to SG_ALERT_COLOR | ||
| self.ui.context_widget.ui.task_label.setStyleSheet( |
There was a problem hiding this comment.
Like I mentioned bellow, I'm not sure it's a good idea to go via the context_widget.ui, the widgets in there might get updated. Though it looks like this particular label is not exposed publicly.
I guess the correct thing to do would be to update the qtwidgets framework to publicly expose that label.
@jfboismenu what do you think?
| # Check for task_required and change UI styling accordingly | ||
| if not self._validate_task_required(): | ||
| # Change task label color to SG_ALERT_COLOR | ||
| self.ui.context_widget.ui.task_label.setStyleSheet( | ||
| "color: " | ||
| + sgtk.platform.current_bundle().style_constants["SG_ALERT_COLOR"] | ||
| ) | ||
| # Also change the text and color of the label | ||
| # Use SG_HIGHLIGHT_COLOR for better readability | ||
| context_label_text = ( | ||
| "<p>Task Required is <b>True</b> in your configuration. " | ||
| "Please confirm all Tasks are assigned to continue.</p>" | ||
| ) | ||
| self.ui.context_widget.ui.label.setStyleSheet( | ||
| "color: " | ||
| + sgtk.platform.current_bundle().style_constants[ | ||
| "SG_HIGHLIGHT_COLOR" | ||
| ] | ||
| ) | ||
| else: | ||
| context_label_text = "Task and Entity Link to apply to all items:" | ||
| # Ensure styling overrides are cleared | ||
| self.ui.context_widget.ui.label.setStyleSheet("") | ||
| self.ui.context_widget.ui.task_label.setStyleSheet("") |
There was a problem hiding this comment.
Other than the context_label_text text this looks like a duplication of the code at line 847-873, I wonder if this could be turned into a method?
The previous PR explains the why quite well - #81
This new one addresses an open comment from that PR that was not resolved to use a style_constant instead of a hard-coded color plus it introduces a second re-style of the text above the Task box.