Modernize layout of Fonts & Colors preference page for better alignment and usability by deepika-u · Pull Request #3968 · eclipse-platform/eclipse.platform.ui

deepika-u · 2026-05-05T14:01:50Z

Refactored the layout of the Fonts & Colors preference page to improve alignment and usability.

Aligned filter text field with tree viewer width
Moved expand actions to a toolbar in the right control column next to filter text in the form of an icon.
Added way to collapse actions similar to expand to the toolbar itself in the form of an icon.
Ensured icons are right aligned.
Improved overall spacing and visual structure to match Eclipse UI conventions similar to Preferences -> Java -> Compiler -> Errors/Warnings page.

Before this pr ->

Now with the pr ->

on clicking the collapse all icon, all the sections which are shown expanded gets collapsed.

To maintain consistency across other preference pages like ->

github-actions · 2026-05-05T15:01:57Z

Test Results

852 files ±0 852 suites ±0 51m 34s ⏱️ -17s
7 940 tests ±0 7 697 ✅ ±0 243 💤 ±0 0 ❌ ±0
20 322 runs ±0 19 667 ✅ ±0 655 💤 ±0 0 ❌ ±0

Results for commit ad475cd. ± Comparison against base commit 7f682c2.

♻️ This comment has been updated with latest results.

deepika-u · 2026-05-05T18:17:08Z

@vogella - Can you take a look at this when you get some time please?

vogella · 2026-05-06T06:54:58Z

@deepika-u I have the problem that the expand buttons are IMHO the ugliest buttons we have in Eclipse. Everytime I see these ugly button gradients I feel the desire to remove them.

So I think I 'm not the right person to review this

merks

Overall it looks good, but I've only looked at the changes here, not actually running in the IDE.

The setFont changes concern me a bit because I don't know the impact and you've not shown pictures of the impact.

merks · 2026-05-06T07:18:05Z

+	 *
+	 * @since 4.5
+	 */


It's not API so no need for @since.

oh ok removed them, thank you.

merks · 2026-05-06T07:18:45Z

 				appliedDialogFont.dispose();
 			}
+	        // Dispose custom images
+	        if (expandImage != null && !expandImage.isDisposed()) {


I believe that you don't need an isDisposed guard just as there isn't one on the appliedDialogFont.

updated as per appliedDialogFont.

merks · 2026-05-06T07:20:03Z

 		Composite mainColumn = new Composite(advancedComposite, SWT.NONE);
-		GridLayout layout = new GridLayout();
-		layout.numColumns = 2;
+		GridLayout layout = new GridLayout(2, false);


You're inlining, which is fine, but as a reviewer it's not easy to see that this is identical. I trust that it is.

Thanks for calling that out. Yes, this is functionally identical - new GridLayout(2, false) is equivalent to setting numColumns = 2 with the default makeColumnsEqualWidth = false.

I agree that the previous form may be easier to compare in a diff. I can revert to the expanded version if you prefer for readability.

Actually it's more an observation. I like it simpler the way you wrote it! I trust that you are properly testing all the layout and appearance that the simpler expression produces good behavior.

merks · 2026-05-06T07:20:42Z

+		GridLayout layout = new GridLayout(2, false);
 		layout.marginWidth = 0;
 		layout.marginHeight = 0;
-		mainColumn.setFont(parent.getFont());


Why this change? This might have an impact that I don't notice.

Good point — thanks for calling that out.

This line was removed during layout refactoring, but it’s not strictly required since SWT controls inherit the parent font by default.

That said, I agree that keeping it makes the intent explicit and avoids any platform-specific inconsistencies. Shall i restore it to be safe and keep behavior identical to the original implementation.

Yes, better safe than sorry I think.

merks · 2026-05-06T07:23:04Z

+		// LOAD IMAGES ONCE
+	    ImageDescriptor expandDesc = ResourceLocator
+				.imageDescriptorFromBundle("org.eclipse.ui", "icons/full/elcl16/expandall.svg") //$NON-NLS-1$ //$NON-NLS-2$
+	        .orElse(null);


If this were to fail, the button will be useless (horribly ugly) won't it?

ImageDescriptor collapseDesc = ResourceLocator .imageDescriptorFromBundle("org.eclipse.ui", "icons/full/elcl16/collapseall.svg") //$NON-NLS-1$ //$NON-NLS-2$ .orElse(null); collapseAllItem = new ToolItem(treeToolbar, SWT.PUSH); if (collapseDesc != null) { collapseImage = collapseDesc.createImage(); collapseAllItem.setImage(collapseImage); } else { // Fallback to shared image collapseAllItem.setImage( PlatformUI.getWorkbench().getSharedImages() .getImage(org.eclipse.ui.ISharedImages.IMG_ELCL_COLLAPSEALL)); } collapseAllItem.setToolTipText("Collapse All"); //$NON-NLS-1$

Thought of this way. But in the similar lines, icon for Expand All is not readily available in org.eclipse.ui.ISharedImages, how should this be handled in your opinion? can i use org.eclipse.ui.ISharedImages.IMG_ELCL_COLLAPSEALL or IMG_ELCL_REMOVE?

I think in the past the tendency was to ensure that the icon was available locally in the bundle rather than to use fragile programmatic access.

Let's ask @HeikoKlare @BeckerWdf how one can best do something here that's not fragile.

you could simply copy the PNG files into the bundle.
Alternatively add "Expand All" as org.eclipse.ui.ISharedImages as API and use it.

Sorry, let me correct my update and put it across again in some time.

Thanks for the insights @BeckerWdf and @merks
I tried exploring that way too but was unsure how to achieve it correct and complete.

My findings are : icons/images are already existing in below paths.

..\git\eclipse.platform.ui\bundles\org.eclipse.ui.ide\icons\full\elcl16 but here expandall.svg/png are not available.

..\git\eclipse.platform.ui\bundles\org.eclipse.ui.editors\icons\full\elcl16 has both.

..\git\eclipse.platform.ui\bundles\org.eclipse.ui\icons\full\elcl16 has both.

..\git\eclipse.platform.ui\bundles\org.eclipse.ui.navigator\icons\full\elcl16 but here expandall.svg/ png are not available.

So for this pr the respective java file is at "..\git\eclipse.platform.ui\bundles\org.eclipse.ui.workbench\eclipseui\org\eclipse\ui\internal\themes\ColorsAndFontsPreferencePage.java"

Option 1 - Add respective expandall, collapseall icons svg/png at freshly created path for this plugin "..\git\eclipse.platform.ui\bundles\org.eclipse.ui.workbench\icons\full\elcl16" and use like below

ImageDescriptor expandDesc = AbstractUIPlugin.imageDescriptorFromPlugin(
"org.eclipse.ui.workbench",
"icons/full/elcl16/expandall.svg"
);

expandImage = expandDesc.createImage();
expandAllItem.setImage(expandImage);

Option 2 - For API way, if i add "icons/full/elcl16/expandall.svg" corresponding entries(inline with collapseall.svg which is already accessible) in ISharedImages.java and WorkbenchImages.java. Are these the only 2 steps i need to perform or do i have to add anything else which i might have missed.

Another observation as well.
I also see entries in plugin.xml at "..\git\eclipse.platform.ui\bundles\org.eclipse.ui" as below

<image commandId="org.eclipse.ui.navigate.collapseAll" icon="$nl$/icons/full/elcl16/collapseall.svg"> </image> <image commandId="org.eclipse.ui.navigate.expandAll" icon="$nl$/icons/full/elcl16/expandall.svg"> </image>

So anything in similar lines needs to be done? But I do understand these are used by the command framework and are not directly applicable for ToolItem usage but better to do, so that it is available as a generic solution if anywhere else - it can also be used(since collapseall is already available but not expandall)
Please let me know if my understanding is correct. Thanks in advance.

merks · 2026-05-06T07:25:45Z

@@ -974,7 +1001,8 @@ private String getText(Object element) {

 		tree = new FilteredTree(parent, SWT.SINGLE | SWT.H_SCROLL | SWT.V_SCROLL | SWT.BORDER, filter, true);


It might be nice to fix this given so many other changes here.

Updated it to tree = new FilteredTree(parent, SWT.SINGLE | SWT.H_SCROLL | SWT.V_SCROLL | SWT.BORDER, filter, true, true);
I’ve updated the deprecated constructor to the newer variant using the additional parameter for fast lookup. This keeps the behavior consistent while removing the warning.

deepika-u · 2026-05-06T07:46:27Z

@deepika-u I have the problem that the expand buttons are IMHO the ugliest buttons we have in Eclipse. Everytime I see these ugly button gradients I feel the desire to remove them.

So I think I 'm not the right person to review this

@vogella
Thanks for the feedback — I understand your concern about the visual style of those buttons.

In this change, the goal wasn’t to introduce any new visual design, but to improve layout consistency and usability (alignment of the filter, tree, and action controls). The icons used are existing Eclipse resources, so this mainly repositions current actions and adds the complementary expand/collapse functionality that was missing.

merks · 2026-05-06T07:57:12Z

I understand your concern about the visual style of those buttons.

Indeed. We are all aware that other folks are busily creating a new dual tone icon pack so one might imagine a pretty icon instead so as better to focus on the design and functionality.

vogella · 2026-05-06T08:00:07Z

@deepika-u all good, just wanted to explain why I don't review this one

deepika-u · 2026-05-06T08:24:51Z

all good, just wanted to explain why I don't review this one

Got it, thanks for clarifying.
Appreciate the context!

Replace expand all button with icons right aligned.

781c9f7

merks requested changes May 6, 2026

View reviewed changes

Updated as per review comments.

ad475cd

deepika-u force-pushed the modernize_expandallbutton branch from fe7a8c8 to ad475cd Compare May 6, 2026 08:54

		@@ -974,7 +1001,8 @@ private String getText(Object element) {

		tree = new FilteredTree(parent, SWT.SINGLE \| SWT.H_SCROLL \| SWT.V_SCROLL \| SWT.BORDER, filter, true);

Conversation

deepika-u commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

deepika-u commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vogella commented May 6, 2026

Uh oh!

merks left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deepika-u May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deepika-u commented May 6, 2026

Uh oh!

merks commented May 6, 2026

Uh oh!

vogella commented May 6, 2026

Uh oh!

deepika-u commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

deepika-u commented May 5, 2026 •

edited

Loading

github-actions Bot commented May 5, 2026 •

edited

Loading

deepika-u commented May 5, 2026 •

edited

Loading

deepika-u May 6, 2026 •

edited

Loading

deepika-u commented May 6, 2026 •

edited

Loading