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

Bad UI on MacOS for merge viewer #1173

Open
zulus opened this issue Sep 26, 2023 · 19 comments · May be fixed by #2629
Open

Bad UI on MacOS for merge viewer #1173

zulus opened this issue Sep 26, 2023 · 19 comments · May be fixed by #2629
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@zulus
Copy link
Contributor

zulus commented Sep 26, 2023

I'm able to reproduce with any viewer, for example java:

  1. Rename file in Project Explorer
  2. Show change preview
  3. Switch between files via mouse (via keyboard works fine), not always appear on first time

Result on screen, viewer is extremely small (see screen)
Screenshot 2023-09-26 at 12 40 33

@iloveeclipse iloveeclipse added help wanted Extra attention is needed bug Something isn't working labels Sep 26, 2023
@iloveeclipse
Copy link
Member

Is this a regression? If yes, in which version?

Note: there aren't many contributors with Mac access, so if you can reproduce & fix, patch is welcome.

@zulus
Copy link
Contributor Author

zulus commented Sep 26, 2023

Is this a regression? If yes, in which version?

Regression definitely, but at least 4 releases, today this just driving me crazy :P (I saw this on JDT, PDT and GenericEditor)

Note: there aren't many contributors with Mac access, so if you can reproduce & fix, patch is welcome.

First I have to find correct UI classes

In general I have a lot of small but sometimes annoying UI issues on MacOS, I'll see what I can do ;)

@iloveeclipse
Copy link
Member

First I have to find correct UI classes

CompareEditor, StructureDiffViewer, DiffTreeViewer

@vogella
Copy link
Contributor

vogella commented Sep 26, 2023

Cool to see someone else using the dark theme @zulus Thanks for looking into this issue.

@zulus
Copy link
Contributor Author

zulus commented Sep 26, 2023

CompareEditor, StructureDiffViewer, DiffTreeViewer

Look like problem is earlier, on SWT Tree:

When works correctly I see SWT events:

  • down
  • select
  • up

Is broken when eclipse receive

  • down
  • up
  • mouse exit (why???)
  • up (duplicated event)
  • select

I have no idea why :( Display sends exit event because Display.findControl return null (this is checked inside applicationSendTrackingEvent)

@akurtakov
Copy link
Member

@Phillipus maybe you can manage to give some hints here

@Phillipus
Copy link
Contributor

Phillipus commented Sep 26, 2023

@Phillipus maybe you can manage to give some hints here

I can't reproduce this.

I'm on macOS Ventura M1 Silicon with hi-dpi screen. Eclipse 4.28, Java 17.

Maybe I'm missing a step to reproduce this?

@Phillipus
Copy link
Contributor

Phillipus commented Sep 26, 2023

Update. I tried on my MacBook Air, Intel, macOS Ventura with track pad and I'm seeing this too. Not sure if this is an Intel vs Silicon thing or a mouse vs trackpad thing. Resizing the dialog window forces proper display.

Edit: Can now see this behaviour on Mac M1 with mouse.

@zulus
Copy link
Contributor Author

zulus commented Sep 27, 2023

@zulus It would help if you could provide more details for macOS version, Mac version, Eclipse version, Java version etc...

Java 17 Temurin, Ventura 13.5.2 (m1 max), Eclipse 2023-09, I see this with magic mouse and trackpad

@Phillipus
Copy link
Contributor

Java 17 Temurin, Ventura 13.5.2 (m1 max), Eclipse 2023-09, I see this with magic mouse and trackpad

Thanks. Since my previous messages I'm also able to reproduce this with mouse or trackpad on Intel and Mac Silicon.

@zulus
Copy link
Contributor Author

zulus commented Sep 27, 2023

I think I understand what happens:

  1. Mouse down (if don't up immediately you will always see this behaviour until up), it firesSelectionEvent
  2. Select has been sent (if you keep mouse clicked will see same as on screen)
  3. Mouse up (normally during this layout is relcalculated (event open), but if it hapen before select finish his job "up" event isn't able to detect view, because durin changing page, focus is updated, so "mouse select"/open have invalid data

Its due busy indicator, starts always during feedInput when documents are created (it makes user interface inactive for a moment, so all layouts are suspended and wrongly calculated). If you move feedInput after showPage, UI looks a little bit better but it's still unusable.

@zulus
Copy link
Contributor Author

zulus commented Oct 4, 2023

I implemented workaround on LTK level: eclipse-jdt/eclipse.jdt.ui#844

@iloveeclipse
Copy link
Member

I implemented workaround on LTK level: eclipse-jdt/eclipse.jdt.ui#844

I believe this is not the right place to fix such (OS specific) issues. I would assume this is something on lower level, like SWT or Platform UI.

@zulus
Copy link
Contributor Author

zulus commented Oct 4, 2023

To be honest, platform issue is suspended layout. But in general, IChangePreviewViewers aren't correctly created, their merge viewers aren't connected to ICompareContainer, so in moment when DocumentMergeViewer prepare ranges during "doDiff" extra popup is created

@zulus
Copy link
Contributor Author

zulus commented Oct 6, 2023

OK, so now platform lvl fix eclipse-platform/eclipse.platform#732 ;)

@zulus
Copy link
Contributor Author

zulus commented Oct 10, 2023

PR merged

@zulus zulus closed this as completed Oct 10, 2023
@iloveeclipse
Copy link
Member

@iloveeclipse iloveeclipse reopened this Nov 23, 2023
@BeckerWdf
Copy link
Contributor

@tobiasmelcher:
Does your PR eclipse-platform/eclipse.platform#1625 fix this UI glitch?

@tobiasmelcher
Copy link
Contributor

eclipse-platform/eclipse.platform#1625

No, org.eclipse.ltk.internal.ui.refactoring.PreviewWizardPage needs to be modified the similar way like in #1625 . Method showPreview cannot be called on selectionChanged but needs to be delayed to onMouseUp. I can do the change if you want.

@tobiasmelcher tobiasmelcher linked a pull request Dec 14, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants