On Tue, 11 Feb 2025 03:11:18 GMT, Prasanta Sadhukhan <psadhuk...@openjdk.org> wrote:
>>> I am checking for WIndows L&F so Metal will not be affected. >> >> This is the reason why I don't like this approach: the base class needs to >> know about Windows L&F and do some customisation based on the current L&F. >> This code will be included on Linux and macOS where Windows L&F isn't >> available. >> >> A better solution would be a helper method or a helper class which handles >> this difference. >> >> As far as I can see, Metal L&F paints both the bullet / check-mark and the >> custom icon — this is exactly what you're implementing here. The difference >> is in margins around these elements. >> >> The layout of the menu is handled by `MenuItemLayoutHelper`. It's customised >> for Synth L&F in `SynthMenuItemLayoutHelper`. Perhaps, you need to create a >> customised class for Windows L&F. >> >> Yet there's no hook in `BasicMenuItemUI` to create or pass another >> `MenuItemLayoutHelper` object which customises the layout. A new method may >> be added. >> >> --- >> >> I haven't looked deeply into how popup menus are laid out and therefore I >> can't advice on how to customise menu item layout. > > It is just checking for current L&F is Windows or not..I am not even checking > for "instanceof WIndowsLookAndFeel" which will not be present in linux and > mac so I dont think it will create any problem.. > Helper method will create unnecessary changes in all L&F..I dont think I will > like to do that.. It just feels wrong… A superclass doesn't need to know, shouldn't know, about its subclasses. Yet this solution could be the simplest one… if achieving the same effect different makes the code too complicated. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r1951015048
