NextButton not disabled toggling "Hide items that are already installed"#1048
Conversation
|
Please check the parent issue for more details |
subyssurendran666
force-pushed
the
next_button_being_enabled_1036
branch
from
6c4796e to
b0dfa9e
Compare
|
@laeubi Could you please trigger the GitHub Copilot review for this PR? |
merks
left a comment
merks
left a comment
There was a problem hiding this comment.
I wonder, is it really necessary to use asyncExec? Better in general to get the display from a widget in the context.
subyssurendran666
force-pushed
the
next_button_being_enabled_1036
branch
from
b0dfa9e to
7c59010
Compare
merks
left a comment
merks
left a comment
There was a problem hiding this comment.
Wow, this code is kind of terrible to begin with before you improved it. Sorry I didn't look closer before.
There appear to be three identical listeners each with two methods that have identical bodies, so like 6 copies in all! Better would be to create one listener (adding it 3 times to the separate controls) and to de-duplicate the identical bodies. Sorry, I know you didn't make it ugly like this, but it would sure be nice to improve it.
Test Results 18 files 18 suites 29m 27s ⏱️ Results for commit ea194b6. ♻️ This comment has been updated with latest results. |
subyssurendran666
force-pushed
the
next_button_being_enabled_1036
branch
from
7c59010 to
32ddad6
Compare
I had a reason for using asyncExec here. In this case it is still required because the viewer state (checked elements) is updated after update available ViewState, so a synchronous call reads stale data. Using asyncExec ensures the UI refresh is complete before evaluating the selection state. Could you please suggest an alternative approach to handle this more cleanly? |
merks
left a comment
merks
left a comment
There was a problem hiding this comment.
This looks so much better❣️
|
Is the failure
perhaps related to this change |
|
FYI, I have this in the works hopefully to fix PR build failures: |
|
I'll rebase to get a cleaner build. |
Toggling "Hide items that are already installed" refreshes the available software view, but the wizard does not update the Next button state. This change ensures the Next button is disabled when no installable units are selected. fix: eclipse-equinox#1036
merks
force-pushed
the
next_button_being_enabled_1036
branch
from
32ddad6 to
ea194b6
Compare
2 participants
Toggling "Hide items that are already installed" refreshes the available software view, but the wizard does not update the Next button state. This change ensures the Next button is disabled when no installable units are selected.
fix: #1036