Skip to content

Newer Workspace Version popup dialog ui alignment changes. #2949

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Newer Workspace Version popup dialog ui alignment changes. #2949

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion ...clipse.ui.ide.application/src/org/eclipse/ui/internal/ide/application/IDEApplication.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2003, 2020 IBM Corporation and others.
* Copyright (c) 2003, 2025 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand Down Expand Up @@ -56,6 +56,8 @@
import org.eclipse.osgi.util.NLS;
import org.eclipse.swt.SWT;
import org.eclipse.swt.layout.FillLayout;
import org.eclipse.swt.layout.GridData;
import org.eclipse.swt.widgets.Button;
import org.eclipse.swt.widgets.Composite;
import org.eclipse.swt.widgets.Control;
import org.eclipse.swt.widgets.Display;
Expand Down Expand Up @@ -727,6 +729,17 @@ protected ReturnCode checkValidWorkspace(Shell shell, URL url) {
MessageDialogWithToggle dialog = new MessageDialogWithToggle(shell, title, null, message, severity,
buttonLabelToId, 0, IDEWorkbenchMessages.IDEApplication_version_doNotWarnAgain, false) {
@Override
protected Control createDialogArea(Composite parent) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this the right place to fix this. Shouldn't we fix this in MessageDialogWithToggle?
So that for all the dialogs inheriting from this class the checkbox is aligned with the text and not with the icon?

Copy link
Contributor Author

@deepika-u deepika-u May 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this the right place to fix this. Shouldn't we fix this in MessageDialogWithToggle?

Checking on this, will update you soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @BeckerWdf
I tried updating the method createToggleButton() as below in MessageDialogWithToggle.java(leaving createDialogArea() untouched) but it is not being honoured to correct the indentation of check box as required.

	GridData data = new GridData(SWT.BEGINNING, SWT.TOP, false, false);
	data.horizontalIndent = 50;

Do you have any other thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BeckerWdf : Can we merge this fix like this? if you dont have any further suggestions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BeckerWdf : When you get some free time, can you check on this and if no further suggestions - can we merge this please?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for missing this.
MessageDialogWithToggle#createToggleButton

looks like this:

	protected Button createToggleButton(Composite parent) {
		final Button button = new Button(parent, SWT.CHECK | SWT.LEFT);

		GridData data = new GridData(SWT.NONE);
		data.horizontalSpan = 2;
		button.setLayoutData(data);
		button.setFont(parent.getFont());

		button.addSelectionListener(widgetSelectedAdapter(e -> toggleState = button.getSelection()));

		return button;
	}

Wouldn't be more consistent from a UI point of view to add your changes there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried at this place as per my previous comment but didnt help. -> #2949 (comment)

To be specific i have tried as below::

	protected Button createToggleButton(Composite parent) {
		final Button button = new Button(parent, SWT.CHECK | SWT.LEFT);

		GridData data = new GridData(SWT.BEGINNING, SWT.TOP, false, false);
		data.horizontalSpan = 2;
		data.horizontalIndent = 50;
		button.setLayoutData(data);
		button.setFont(parent.getFont());
		button.addSelectionListener(widgetSelectedAdapter(e -> toggleState = button.getSelection()));
		return button;
	}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why this did not help?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As stated above, the same change i have put in the 4.35 maintenance branch and built a complete eclipse local build. Installed and launched that eclipse to open a workspace used by 4.36 till then but the alignment is not corrected as per the code. The same is my observation in -> #2949 (comment)

Composite composite = (Composite) super.createDialogArea(parent);
Button toggle = getToggleButton();
if (toggle != null && !toggle.isDisposed()) {
GridData toggleData = new GridData(SWT.BEGINNING, SWT.TOP, false, false);
toggleData.horizontalIndent = 50;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you explain this "magic number"?

Copy link
Contributor Author

@deepika-u deepika-u Jun 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was by trial and error, by only which i could arrive to this value to align it with the text(1st warning line text where it starts) in the above line.

For reference -> #2949 (comment)
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May this 50 be wrong on other OSes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have only windows to check on. Not sure though.

toggle.setLayoutData(toggleData);
}
return composite;
}
@Override
protected Shell getParentShell() {
// Bug 429308: Make workspace selection dialog visible
// in the task manager of the OS
Expand Down
Loading