Fix:Layout preference page doesn't keep default layout

The input of the combo viewer needs to be set before the databinding is created. Otherwise the initial selection can't be restored. When unchecking an element in the table, the old selection of the combo viewer needs read before the new input is set, otherwise this information is lost. Fixes eclipse-windowbuilder#785
ptziegler · Jun 3, 2024 · b7b7c51 · b7b7c51
1 parent b8e233c
commit b7b7c51
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 35 deletions.
diff --git a/org.eclipse.wb.core.ui/META-INF/MANIFEST.MF b/org.eclipse.wb.core.ui/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.wb.core.ui;singleton:=true
-Bundle-Version: 1.10.400.qualifier
+Bundle-Version: 1.10.500.qualifier
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
 Require-Bundle: org.eclipse.ui,

diff --git a/...clipse.wb.core.ui/src/org/eclipse/wb/internal/core/preferences/LayoutsPreferencePage.java b/...clipse.wb.core.ui/src/org/eclipse/wb/internal/core/preferences/LayoutsPreferencePage.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2011, 2022 Google, Inc.
+ * Copyright (c) 2011, 2024 Google, Inc. and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -131,7 +131,38 @@ public int compare(LayoutDescription layout_1, LayoutDescription layout_2) {
 						layoutCombo.add(layoutDescription.getName());
 					}
 				}
+				new Label(this, SWT.NONE).setText(UiMessages.LayoutsPreferencePage_availableLayouts);
+				m_table = CheckboxTableViewer.newCheckList(this, SWT.BORDER | SWT.V_SCROLL | SWT.H_SCROLL);
+				m_table.setContentProvider(ArrayContentProvider.getInstance());
+				m_table.setLabelProvider(
+						ColumnLabelProvider.createTextProvider(o -> ((LayoutDescription) o).getName()));
+				m_table.setCheckStateProvider(new ICheckStateProvider() {
+					@Override
+					public boolean isChecked(Object element) {
+						return m_layoutPreferences.getBoolean(((LayoutDescription) element).getLayoutClassName(), true);
+					}
+
+					@Override
+					public boolean isGrayed(Object element) {
+						return false;
+					}
+				});
+				m_table.setInput(layouts);
+				m_table.addCheckStateListener(event -> {
+					LayoutDescription layout = (LayoutDescription) event.getElement();
+					m_layoutPreferences.putBoolean(layout.getLayoutClassName(), event.getChecked());
+					// If default was set to a layout that is de-selected from available layouts.
+					// The default layout is set back to implicit layout
+					List<Object> input = getLayoutItems();
+					Object selection = layoutCombo.getStructuredSelection().getFirstElement();
+					layoutCombo.setInput(input);
+					if (!input.contains(selection)) {
+						layoutCombo.setSelection(implicitLayoutSelection);
+					}
+				});
+				GridDataFactory.create(m_table.getTable()).fillH().spanH(gridLayoutColumns);
 				// bind
+				layoutCombo.setInput(getLayoutItems());
 				m_bindManager.bind(new IDataEditor() {
 					@Override
 					public void setValue(Object value) {
@@ -155,42 +186,11 @@ public Object getValue() {
 						if (layoutCombo.getStructuredSelection()
 								.getFirstElement() instanceof LayoutDescription layout) {
 							return layout.getId();
-						}
+					}
 						// implicit layout
 						return null;
 					}
-				},
-						new StringPreferenceProvider(m_preferences, IPreferenceConstants.P_LAYOUT_DEFAULT), true);
-				new Label(this, SWT.NONE).setText(UiMessages.LayoutsPreferencePage_availableLayouts);
-				m_table = CheckboxTableViewer.newCheckList(this, SWT.BORDER | SWT.V_SCROLL | SWT.H_SCROLL);
-				m_table.setContentProvider(ArrayContentProvider.getInstance());
-				m_table.setLabelProvider(ColumnLabelProvider.createTextProvider(o -> ((LayoutDescription)o).getName()));
-				m_table.setCheckStateProvider(new ICheckStateProvider() {
-					@Override
-					public boolean isChecked(Object element) {
-						return m_layoutPreferences.getBoolean(((LayoutDescription) element).getLayoutClassName(), true);
-					}
-
-					@Override
-					public boolean isGrayed(Object element) {
-						return false;
-					}
-				});
-				m_table.setInput(layouts);
-				m_table.addCheckStateListener(event -> {
-					LayoutDescription layout = (LayoutDescription) event.getElement();
-					m_layoutPreferences.putBoolean(layout.getLayoutClassName(), event.getChecked());
-					// If default was set to a layout that is de-selected from available layouts.
-					// The default layout is set back to implicit layout
-					List<Object> layoutItems = new ArrayList<>();
-					layoutItems.add(UiMessages.LayoutsPreferencePage_implicitLayout);
-					layoutItems.addAll(List.of(m_table.getCheckedElements()));
-					layoutCombo.setInput(layoutItems.toArray());
-					if (layoutCombo.getStructuredSelection().isEmpty()) {
-						layoutCombo.setSelection(implicitLayoutSelection);
-					}
-				});
-				GridDataFactory.create(m_table.getTable()).fillH().spanH(gridLayoutColumns);
+				}, new StringPreferenceProvider(m_preferences, IPreferenceConstants.P_LAYOUT_DEFAULT), true);
 			}
 			// boolean preferences
 			checkButton(
@@ -199,5 +199,12 @@ public boolean isGrayed(Object element) {
 					UiMessages.LayoutsPreferencePage_inheritLayout,
 					IPreferenceConstants.P_LAYOUT_OF_PARENT);
 		}
+
+		private List<Object> getLayoutItems() {
+			List<Object> layoutItems = new ArrayList<>();
+			layoutItems.add(UiMessages.LayoutsPreferencePage_implicitLayout);
+			layoutItems.addAll(List.of(m_table.getCheckedElements()));
+			return Collections.unmodifiableList(layoutItems);
+		}
 	}
 }