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.

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

Conversation

deepika-u
Copy link
Contributor

Before the fix the dialog used to show up as
image

After the fix the dialog now is showed up as
image

This popup dialog basically comes when a workspace for example already used/edited/saved in 4.36 and now attempted to open with an older eclipse say 4.35 or lower, this pop up shows up.

Change 1 : check box is aligned.
Change 2 : Button bar is center aligned.

Here we are changing the behavior of the current popup dialog leaving the original dialog alignments as-is.

@BeckerWdf
Copy link
Contributor

I don't think that center-aligning the button brings an advantage.

@deepika-u
Copy link
Contributor Author

deepika-u commented Apr 29, 2025

I don't think that center-aligning the button brings an advantage.

Thanks for your opinion, will be reverting the 2nd change. Updated as per review comments.

@deepika-u deepika-u force-pushed the newWorkspace_opened_with_oldEclipse branch 2 times, most recently from 91596bb to b45c15e Compare April 29, 2025 07:39
@tomaswolf
Copy link
Member

Yes, don't center the buttons. If at all that would be done in #2929. (And this dialog here might make a good simple test case to check that whatever is done in #2929 doesn't mess up things.)

Copy link
Contributor

github-actions bot commented Apr 29, 2025

Test Results

 1 824 files  ±0   1 824 suites  ±0   1h 35m 8s ⏱️ -31s
 7 918 tests ±0   7 690 ✅ +2  228 💤 ±0  0 ❌  - 2 
23 841 runs  ±0  23 093 ✅ +2  748 💤 ±0  0 ❌  - 2 

Results for commit 399983b. ± Comparison against base commit 912fadf.

♻️ This comment has been updated with latest results.

@deepika-u
Copy link
Contributor Author

Yes, don't center the buttons. If at all that would be done in #2929. (And this dialog here might make a good simple test case to check that whatever is done in #2929 doesn't mess up things.)

Now after the fix, only the check box line is aligned and the buttons will continue to right align as earlier.
From understanding with #2929 and its respective fix only the warning section will be updated but nothing to do with such kind of check box settings given for user to opt. Please correct me if i am not mistaken.

Now after this fix only the 2nd line is aligned changing the behavior of the current popup dialog leaving the original dialog alignments as-is.
image

@deepika-u
Copy link
Contributor Author

@BeckerWdf : can you check this when you have some time please?

@deepika-u deepika-u force-pushed the newWorkspace_opened_with_oldEclipse branch from b45c15e to 399983b Compare May 2, 2025 14:36
@deepika-u
Copy link
Contributor Author

@tomaswolf @BeckerWdf
Do you have any further suggestions regarding this fix?

Here as per the suggestions buttons are right aligned.
Warning is still left aligned.
Only the check box part is aligned. Are we good?

@BeckerWdf
Copy link
Contributor

Aligning the text and the checkbox is fine for me. I know this dialog a lot. I never felt it was wrong. But I think having the checkbox left aligned (with the icon) indeed is not correct.

@@ -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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants