[Win32] Consolidate tool bar image list native synchronization - #3508
Draft
HeikoKlare wants to merge 2 commits into
Draft
[Win32] Consolidate tool bar image list native synchronization #3508HeikoKlare wants to merge 2 commits into
HeikoKlare wants to merge 2 commits into
Conversation
ToolItem manipulated ToolBar's three normal/hot/disabled ImageLists directly: reading them via ToolBar's getters, creating them inline when absent, and calling add()/put() on each of them itself. This spread the bookkeeping for a single item's images across both classes and made ToolItem responsible for details that are really ToolBar's to own, such as lazily creating the image lists sized to the first image added. With this change, ToolBar exposes addImage/putImage/clearImage instead, and ToolItem goes through these instead of touching ImageList directly. ToolBar's internal representation (three separate ImageList fields, and their existing setImageList/setHotImageList/setDisabledImageList synchronization methods) is otherwise unchanged. Related to eclipse-platform#3466
ToolBar's setImageList/setHotImageList/setDisabledImageList each carried a near-identical copy of the logic to compare the current TB_GET*IMAGELIST handle against the ImageList's handle for the current zoom, and, if different, apply it via TB_SET*IMAGELIST while toggling setDropDownItems around it to avoid a Windows layout glitch. handleDPIChange and addImage each called all three setters in turn whenever any one image list might have changed. This change consolidates that duplicated logic into a single refreshImageList/refreshImageLists pair operating on the current field values, replacing the three getters and three setters. This is a standalone, behavior-preserving cleanup; ToolBar's representation is still three separate ImageList fields. It prepares the ground for introducing a class that owns the three image lists as one unit. Related to eclipse-platform#3466
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Important
This change is based on and should thus be merged after #3504
ToolBar'ssetImageList/setHotImageList/setDisabledImageListeach carried a near-identical copy of the logic to compare the currentTB_GET*IMAGELISThandle against the ImageList's handle for the current zoom, and, if different, apply it viaTB_SET*IMAGELISTv while toggling vsetDropDownItemsaround it to avoid a Windows layout glitch.handleDPIChangeandaddImageeach called all three setters in turn whenever any one image list might have changed.This change consolidates that duplicated logic into a single
refreshImageList/refreshImageListspair operating on the currentfield values, replacing the three getters and three setters. This is a standalone, behavior-preserving cleanup;
ToolBar's representation is still three separate ImageList fields. It prepares the ground for introducing a class that owns the three image lists as one unit.Related to #3466
Note: This change is human-crafted and was only slightly revised and documented with the help of AI
This is the second of multiple incremental steps to enhance ImageList handling and consistency inside ToolBar. This is supposed to be merged for 2026-12 M1.