26645: Fixed Undo/Redo buttons' hover state when disabled #26671
+7
−4
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves: #26645
Key points:
&& root.enabled
to the condition for the "HOVERED" state.enabled: root.enabled
from theMouseArea
since it has side effects. See below.&& root.enabled
to the tooltip display so it does not appear when the button is disabled which is the current behavior although there is a story to re-enable tooltips for disabled elements: Make disabled buttons focusable & show their tooltips on hover #26232Disabling the
MouseArea
together with the button has side effects when the button is disabled upon its own click. This is the case with the Undo and Redo buttons when there are no more actions to undo/redo. In particularcontainsMouse
of theMouseArea
remainstrue
when the button and itsMouseArea
get disabled. This means that the "HOVERED" state will remain active. This could be worked around by adding&& mouseArea.enabled
to the condition of the "HOVERED" state.However, there is more:
containsMouse
will continue to betrue
even if the mouse then leaves the now disabled button. Let's say this is the Undo button and it becomes disabled on click since there are no more actions to undo. The user moves the mouse away from it and over the Redo button.containsMouse
of the Undo button'sMouseArea
continues to betrue
since both the button and theMouseArea
are now disabled and theMouseArea
does not react to any mouse events any more. If the Redo button is now clicked, this will re-enable the Undo button and itsMouseArea
. The Undo button's "HOVERED" state will fire even though the mouse is hovering the Redo button.The conclusion for me is we should not be disabling the
MouseArea
. Instead we should be checkingroot.Enabled
where necessary. Again, #26232 is probably another reason to not disable the MouseAreas. The disabling of theMouseArea
-s was done for #25697.@cbjeukendrup What are your thoughts? Should we revert the disabling of all the MouseAreas and add '&& .enabled' where necessary? Or do you see a better approach?