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

Find/Replace Overlay: adapt to target theming #2119

Merged

Conversation

Wittmaxi
Copy link

@Wittmaxi Wittmaxi commented Jul 22, 2024

grafik

The find/replace overlay now gets it's colors from the targetted view if the target is a text editor.

fixes #2118

Before:
grafik

Light theme:
grafik

The find/replace overlay now gets it's colors from the targetted view if
the target is a text editor.

fixes eclipse-platform#2118
@Wittmaxi
Copy link
Author

@thomasritter @BeckerWdf You are closer to the theme revamp than I am. What do you think of this change? Is this the best way to apply your theme to our overaly?

Copy link
Contributor

Test Results

 1 815 files  ±0   1 815 suites  ±0   1h 39m 27s ⏱️ + 5m 2s
 7 679 tests ±0   7 451 ✅ +1  228 💤 ±0  0 ❌  - 1 
24 198 runs  ±0  23 449 ✅ +1  749 💤 ±0  0 ❌  - 1 

Results for commit be8946a. ± Comparison against base commit b502ad7.

@thomasritter
Copy link

Thanks for the heads-up. I will start using it to get a feel for it.

We usually look at other applications for inspirations first. So, I would recommend that you look at:

  • VS Code
  • Intellij
  • Firefox, Chrome
  • Notepad
  • ...

See how they integrated it. Get some inspirations and find out what makes most sense in the Eclipse environment.

@Wittmaxi
Copy link
Author

@thomasritter Do you think it would make sense to merge this as a kind of "dogfooding"-PR and possibly change or revert it later on?

IntelliJ has the search themed to match with the Editor:
grafik

VScode does the same (except vscode has the editor lighter than the outside)
grafik

The windows editor has a "real overlay", I don't think it really applies to our case
grafik

Based on this, I believe that my change is an improvement for the theming. In any case, the new change in theming makes the Overlay stick out like a sore thumb - we should find some solution for it

@vogella
Copy link
Contributor

vogella commented Jul 22, 2024

LGTM

@vogella vogella merged commit 0d6c27b into eclipse-platform:master Jul 22, 2024
16 checks passed
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.

Find/Replace Overlay not adapted to theme of Editor
3 participants