Added dynamic tab alignment support in MultiPageEditorPart

[praveen-skp]

Issue: #2223

This pull request introduces a new feature in Eclipse that allows users to configure the alignment of tabs in multi-page editors. The tabs, which are currently fixed at the bottom, can now be dynamically positioned on the top or bottom, based on user preference.

Key Changes:

• Added a new preference for multi-page editor tab alignment.
• Added a preference change listener to MultiPageEditorPart.
• Updated tab style based on the user's preference.

New preference:

Tabs on top of the screen:

[github-actions]

Test Results

 1 821 files  ±0   1 821 suites  ±0   1h 50m 26s ⏱️ + 1m 33s
 7 724 tests ±0   7 495 ✅  - 1  228 💤 ±0  1 ❌ +1 
24 333 runs  ±0  23 585 ✅  - 1  747 💤 ±0  1 ❌ +1 

For more details on these failures, see this check.

Results for commit 2e123d4. ± Comparison against base commit 2521210.

♻️ This comment has been updated with latest results.

[iloveeclipse]

This is an interesting feature, but ideally it should also give custom multipage editors control whether they want to follow up the tab position or keep some specific tab position independently of the preference, see also coment in code.

[tomaswolf]

... ideally it should also give custom multipage editors control whether they want to follow up the tab position or keep some specific tab position independently of the preference...

Not sure about this. UI consistency is important. If every multipage editor gets to decide on its own, we'll end up with some having the tabs at the top and some at the bottom. Isn't that going to be confusing or at least inconvenient for users?

[iloveeclipse]

Isn't that going to be confusing or at least inconvenient for users?

I'm thinking about products like ours where the editors tab position is important because we have lot of complicated and nested UI elements, so users "occasionally" changing global preference would see "broken" UI's in the places where nothing was changed for years.

[praveen-skp]

Hi @iloveeclipse, Hi @laeubi ,
I have incorporated the review comments. Could you please go through the change once again ?

[BeckerWdf]

New preference:

I wondering if we should move the new checkbox above the "Number of open editor..." setting so that all checkboxes are next to each other?
Did you move it below the "Number of open editor..." by intention?

[praveen-skp]

Sorry, i forgot to add the current UI after the change. Its now changed from check box to dropdown, after incorporating the review comments.

[BeckerWdf]

Sorry, i forgot to add the current UI after the change. Its now changed from check box to dropdown, after incorporating the review comments.

Ah yes. Forgot that. In that case I think the ordering is ok. And I just realised that the "Automatically close.." checkbox and the next input field belong together. One can see this via the indentation. Maybe we could make this more explicit but putting a box around these two - but that would be a different issue.

[BeckerWdf]

but that would be a different issue

Created #2297 for this.

[BeckerWdf]

I'm thinking about products like ours where the editors tab position is important because we have lot of complicated and nested UI elements, so users "occasionally" changing global preference would see "broken" UI's in the places where nothing was changed for years.

I have some doubts: So with the current state the user could set the preference but some individual editor may not consider this. Isn't this strange behaviour from the user's perspective.
I see your use-case but can we find another solution for it? E.g. having a mechanisms for a product definition to hide this new preference so that in your product the user cannot choose to have editor tabs at the top?

[BeckerWdf]

@praveen-skp: Can you pls. rebase an resolve the merge conflicts?

[BeckerWdf]

I see:

03:17:33.152 [WARNING] MavenProject: org.eclipse.platform:org.eclipse.ui.editors:3.18.100-SNAPSHOT @ /home/jenkins/agent/workspace/eclipse.platform.ui_PR-2224/bundles/org.eclipse.ui.editors/.polyglot.META-INF: baseline and build artifacts have same version but different contents

Where does this come from?

[BeckerWdf]

@jukzi:

I see failing test with:

Cannot invoke "java.util.function.Supplier.get()" because "this.shell" is null
java.lang.NullPointerException: Cannot invoke "java.util.function.Supplier.get()" because "this.shell" is null
	at org.eclipse.test.Screenshots$ScreenshotOnFailure.dispose(Screenshots.java:52)
	at org.eclipse.test.Screenshots$ScreenshotOnFailure.finished(Screenshots.java:47)

Can you pls. have a look what's the issue there?

[jukzi]

Please consider rebase/merge of my 2 outstanding PRs to use nonnull Argument to get Screenshots Jörg Kubitz

…

Am 14.10.2024 um 07:36 schrieb Matthias Becker ***@***.***>:  @jukzi: I see failing test with: Cannot invoke "java.util.function.Supplier.get()" because "this.shell" is null java.lang.NullPointerException: Cannot invoke "java.util.function.Supplier.get()" because "this.shell" is null at org.eclipse.test.Screenshots$ScreenshotOnFailure.dispose(Screenshots.java:52) at org.eclipse.test.Screenshots$ScreenshotOnFailure.finished(Screenshots.java:47) Can you pls. have a look what's the issue there? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

[BeckerWdf]

@azoitl: Referring to our talk on OCX.
You said this feature is already available somewhere?

[azoitl]

@azoitl: Referring to our talk on OCX. You said this feature is already available somewhere?

No not yet. There is a bugzilla entry: https://bugs.eclipse.org/bugs/show_bug.cgi?id=58945 there you can find links to the gerrit patches I did. But I do not recommend to use them. There is someone who will start working on that.

[BeckerWdf]

pr-head is failing with

Failed to execute goal org.eclipse.tycho:tycho-surefire-plugin:4.0.9:test (default-test) on project org.eclipse.ui.tests: An unexpected error occurred while launching the test runtime (process returned error code 13). The process logfile /home/jenkins/agent/workspace/eclipse.platform.ui_PR-2224/tests/org.eclipse.ui.tests/target/work/data/.metadata/.log might contain further details. Command-line used to launch the sub-process was /opt/tools/java/openjdk/jdk-17/latest/bin/java -Dosgi.noShutdown=false -Dosgi.os=linux -Dosgi.ws=gtk -Dosgi.arch=x86_64 --add-modules=ALL-SYSTEM -Dosgi.clean=true -ea -jar /home/jenkins/agent/workspace/eclipse.platform.ui_PR-2224/.m2/repository/p2/osgi/bundle/org.eclipse.equinox.launcher/1.6.900.v20240613-2009/org.eclipse.equinox.launcher-1.6.900.v20240613-2009.jar -data /home/jenkins/agent/workspace/eclipse.platform.ui_PR-2224/tests/org.eclipse.ui.tests/target/work/data -install /home/jenkins/agent/workspace/eclipse.platform.ui_PR-2224/tests/org.eclipse.ui.tests/target/work -configuration /home/jenkins/agent/workspace/eclipse.platform.ui_PR-2224/tests/org.eclipse.ui.tests/target/work/configuration -application org.eclipse.tycho.surefire.osgibooter.uitest -testproperties /home/jenkins/agent/workspace/eclipse.platform.ui_PR-2224/tests/org.eclipse.ui.tests/target/surefire.properties in working directory /home/jenkins/agent/workspace/eclipse.platform.ui_PR-2224/tests/org.eclipse.ui.tests -> [Help 1]

This looks unrelated. Should we merge it anyways?

[jukzi]

This looks unrelated. Should we merge it anyways?

Please make sure the CI will not fail after any commit. for that you can rerun the build , document (with an issue) or even fix the failure

[BeckerWdf]

failed test on macOS is already know to fail. it's unrelated See: #370.

If nobody objects I plan to merge this today.