Skip to content
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

Improve UI Indication for Unsaved Changes in Tabs #1632

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

schneidermic0
Copy link

@schneidermic0 schneidermic0 commented Dec 2, 2024

Before this change: If a part is dirty (has unsaved changes), this will be indicated by an * in front of a tab's name.

With this change, the * is removed and dirty parts are indicated by a bullet at the location of closing a tab. As soon as, the bullet is hovered, the "button" to close the tab is shown.

Furthermore, the rendering of the close button of tabs is lightly changed.

Examples

Tabs unchanged:
image

Tabs unchanged (hovered):
image

Tab "Test.java" (dirty; selected):
image

Tab "Test.java" (dirty; not selected):
image

Tab "Test.java" (dirty; hovered)
image

Complete flow:
2024-12-02_20-52-27

To enable this behavior, changes in PR eclipse-platform/eclipse.platform.ui#2568 are required, too. However, this change can be merged first.

Tabs render dirty parts by showing a `*` in front of the tab name (e.g.,
in front of the file name.

This information is hard to see by developers. This change introduces a
graphical indicator on the close button to highlight dirty (unsaved)
changes.
@schneidermic0
Copy link
Author

Hi,

I have opened this PR as draft, because I want to get your opinion on this change. The code can be improved at several places and current implementation has some shortcomings that need to be improved as soon as this approach shall be taken up.

Existing gaps (as of now):

  • Dirty indicator is not moved to not, yet opened tab (i.e, if there are two tabs of the same file)
  • In the tab overview and editor selection (Ctrl+E), dirty editors are still marked with *

@schneidermic0 schneidermic0 changed the title Feature/new dirty indicator bullet Improve UI Indication for Unsaved Changes in Tabs Dec 2, 2024
@vogella
Copy link
Contributor

vogella commented Dec 2, 2024

Looks awesome to me, thanks. The * is outside Eclipse not well understood as "I need saving". By modifying the close button in case of an editor which need saving we clearly indicate that close is not different. Other editors like VScode use the same approach.

Copy link
Contributor

github-actions bot commented Dec 2, 2024

Test Results

80 files   -    403  80 suites   - 403   2s ⏱️ - 8m 37s
34 tests  -  4 061  34 ✅  -  4 051  0 💤  -  7  0 ❌  - 3 
34 runs   - 16 139  34 ✅  - 16 046  0 💤  - 90  0 ❌  - 3 

Results for commit ff3d557. ± Comparison against base commit 2ce8542.

This pull request removes 4061 tests.
org.eclipse.swt.graphics.ImageWin32Tests ‑ testImageShouldHaveDimesionAsPerZoomLevel
org.eclipse.swt.tests.junit.DPIUtilTests ‑ scaleDownFloat
org.eclipse.swt.tests.junit.DPIUtilTests ‑ scaleDownFloatArray
org.eclipse.swt.tests.junit.DPIUtilTests ‑ scaleDownInteger
org.eclipse.swt.tests.junit.DPIUtilTests ‑ scaleDownPoint
org.eclipse.swt.tests.junit.DPIUtilTests ‑ scaleDownRectangle
org.eclipse.swt.tests.junit.DPIUtilTests ‑ scaleUpFloat
org.eclipse.swt.tests.junit.DPIUtilTests ‑ scaleUpIntArray
org.eclipse.swt.tests.junit.DPIUtilTests ‑ scaleUpInteger
org.eclipse.swt.tests.junit.DPIUtilTests ‑ scaleUpPoint
…

♻️ This comment has been updated with latest results.

@iloveeclipse
Copy link
Member

Other editors like VScode use the same approach

Could you attach some screenshots from "other" editors? I haven't seen yet something like proposed here.

@@ -877,44 +877,58 @@ void drawBody(GC gc, Rectangle bounds, int state) {
}
}

void drawClose(GC gc, Rectangle closeRect, int closeImageState) {
void drawClose(GC gc, Rectangle closeRect, int closeImageState, boolean showDirtyIndicator) {
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this code, and, in general, tab presentation (font style, color, text decoration, borders, close icon) could be delegated to a new callback interface that, if the callback is not set, would by default use current SWT implementation, and if set, would give the callback full control of the tab visuals.
This way SWT could keep its code independent of the application needs and application (like workbench renderers) could implement whatever possible.

I personally would like to see dirty tabs in bold red font and I don't want to see close buttons at all to save space. I've had that in my Extended VS Presentation plugin for Eclipse 3.x many years ago before e4 killed Presentation API.
That's of course much more as proposal here, but it shouldbe doable and it would allow 3rd parties to decide whether they want close buttons turned into circles or disappear completely or just stay "old good" black crosses etc.

Copy link
Member

Choose a reason for hiding this comment

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

@BeckerWdf
Copy link
Contributor

Hi,

I have opened this PR as draft, because I want to get your opinion on this change. The code can be improved at several places and current implementation has some shortcomings that need to be improved as soon as this approach shall be taken up.

Existing gaps (as of now):

  • Dirty indicator is not moved to not, yet opened tab (i.e, if there are two tabs of the same file)
  • In the tab overview and editor selection (Ctrl+E), dirty editors are still marked with *

What about tabs of multi-page editors as e.g. the plugin.xml editor?

@vogella
Copy link
Contributor

vogella commented Dec 3, 2024

Other editors like VScode use the same approach

Could you attach some screenshots from "other" editors? I haven't seen yet something like proposed here.

image

@schneidermic0
Copy link
Author

What about tabs of multi-page editors as e.g. the plugin.xml editor?

In my IDE, the tabs of multi-page editors (e.g. MANIFEST.MF) don't show an indicator.

@BeckerWdf I believe you know the *-indicators in tabs of multi-page editors in ABAP development tools. These need to be addressed separately.

@laeubi
Copy link
Contributor

laeubi commented Dec 3, 2024

Looks awesome to me, thanks. The * is outside Eclipse not well understood as "I need saving".

I think this is maybe a biased opinion, at least on Debian / Gnome this is common and "well understood" (at least by me)

grafik

Notepad++ (windows) uses a red icon

grafik

So as always there is not the one-and-only truth ;-)

I personally would like to see dirty tabs in bold red font and I don't want to see close buttons at all to save space.

I think one should be able to change this via CSS styling.

@iloveeclipse
Copy link
Member

Can't one change this via CSS styling.

Yes, but I don't also like native widgets to be additionally styled by e4 css for multiple reasons.

@laeubi
Copy link
Contributor

laeubi commented Dec 3, 2024

Can't one change this via CSS styling.

Yes, but I don't also like native widgets to be additionally styled by e4 css for multiple reasons.

CTabFolder is not a native widget ;-)

@iloveeclipse
Copy link
Member

CTabFolder is not a native widget ;-)

But the rest of the UI is, and you can't have only tabs styled as of today.

@BeckerWdf
Copy link
Contributor

what I like with the proposed change is that the black circle is much bigger then the little tiny "*" we use today.
And also the width of the tab is not changing when the editor is dirty so it does not "flicker".

@schneidermic0
Copy link
Author

Thank you for the feedback so far. I've noticed some positive responses, but I’m uncertain about the rest. Should I proceed with improving this PR, or do we need more input from the community first?

I can work on it when I find some time alongside my other tasks.

@vogella
Copy link
Contributor

vogella commented Dec 4, 2024

@HannesWell and @akurtakov please bring this to the PMC and decide if this is desired

@HannesWell
Copy link
Member

@HannesWell and @akurtakov please bring this to the PMC and decide if this is desired

We have discussed this topic in the open dev-call on Thursday:
https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/wiki/Eclipse-IDE-%E2%80%90-Developers-Community-Call#06th-december-2024

In general the feedback was that avoiding resizing is nice, but the exact 'dirty' symbol might not be liked by everyone and maybe alternatives could be considered. Furthermore the old and new behavior would ideally be configurable in a preference.

@mickaelistria
Copy link
Contributor

what I like with the proposed change is that the black circle is much bigger then the little tiny "*" we use today.

For that particular part, just using another character such as or even ✏️ char instead of * could do the job; or maybe even keeping the * but making it bold. I personally like emojis as they are very expressive and are undertood more easily; the drawback is that there is a trend of color-less and fade icons while emojis tend to be the opposite.

And also the width of the tab is not changing when the editor is dirty so it does not "flicker".

I also find it interesting. However it is un

Another possibility that would fit both could be an overlay on the editor icon of the tab, but it might be more complex to implement.

@schneidermic0
Copy link
Author

@HannesWell Thanks for the update. Next step: I will add a preference so that users can enable the new behavior.

@schneidermic0
Copy link
Author

For that particular part, just using another character such as ● or even ✏️ char instead of * could do the job; or maybe even keeping the * but making it bold. I personally like emojis as they are very expressive and are undertood more easily; the drawback is that there is a trend of color-less and fade icons while emojis tend to be the opposite.

Actually, using an emoji was my first attempt. Unfortunately this changed the size of the tab much more than the *. Therefore, I tried it with the change I proposed with this PR.

@schneidermic0
Copy link
Author

And also the width of the tab is not changing when the editor is dirty so it does not "flicker".

I also find it interesting. However it is un

@mickaelistria I guess there is something missing in your comment, isn't it?

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.

7 participants