| Summary: | Refactoring Wizard's Preview shows light background in dark theme | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Matthias Becker <ma.becker> | ||||||
| Component: | UI | Assignee: | Matthias Becker <ma.becker> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | daniel_megert, Lars.Vogel, noopur_gupta, sarika.sinha, twolf | ||||||
| Version: | 4.9 | ||||||||
| Target Milestone: | 4.10 M3 | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| See Also: |
https://git.eclipse.org/r/127942 https://bugs.eclipse.org/bugs/show_bug.cgi?id=533879 https://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=0c40c1089b8338ffe690c09c4be80401d9ddda92 |
||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
New Gerrit change created: https://git.eclipse.org/r/127942 Created attachment 275507 [details]
screen shot showing the fix
This is fixed from my side. Can a JDT-Committer pls. review and merge? This change is too big for RC1. Can a JDT committer pls. merge the change? @Thomas: Is there still something to do from my side before you merge it? Thomas, can you please review the change and confirm that the fix works fine for both light and dark themes on all the platforms (Windows, Mac, Linux)? (In reply to Noopur Gupta from comment #7) > Thomas, can you please review the change and confirm that the fix works fine > for both light and dark themes on all the platforms (Windows, Mac, Linux)? I have no development environment set up for working on JDT, so it would take quite a bit of work to even get to the point where I could check out and run this change. I do not have a Windows machine. I have a Mac and two Linux (Ubuntu/CentOS) VMs on that. So: no, I'm sorry, I cannot do that. I can confirm that the setColors() code is the same as we are using in EGit for 3 years now.[1] That's all I can say. [1] https://git.eclipse.org/c/egit/egit.git/tree/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/HyperlinkSourceViewer.java#n334 Thanks, Thomas. Matthias, please update the patch to follow JDT UI coding conventions (https://wiki.eclipse.org/JDT_UI/How_to_Contribute#Coding_Conventions). For example, we use the compact assignment form (a= b, i.e. no space on the left of equals). Also, can you please test and confirm that the fix works fine for both light and dark themes on all the platforms (Windows, Mac, Linux)? (In reply to Noopur Gupta from comment #9) > Thanks, Thomas. > > Matthias, please update the patch to follow JDT UI coding conventions > (https://wiki.eclipse.org/JDT_UI/How_to_Contribute#Coding_Conventions). For > example, we use the compact assignment form (a= b, i.e. no space on the left > of equals). > > Also, can you please test and confirm that the fix works fine for both light > and dark themes on all the platforms (Windows, Mac, Linux)? Dear Noopur, I can confirm this works on macOS as I developed it there. I don't have a windows or linux at hand. Regarding the coding conventions: I am very short on time and have a lot of other (more important stuff to do). So it might be faster that you adapt the patch to match your conventions. Out of time for M1. (In reply to Noopur Gupta from comment #11) > Out of time for M1. from my side it was already ready for the last release (In reply to Noopur Gupta from comment #9) > Thanks, Thomas. > > Matthias, please update the patch to follow JDT UI coding conventions > (https://wiki.eclipse.org/JDT_UI/How_to_Contribute#Coding_Conventions). For > example, we use the compact assignment form (a= b, i.e. no space on the left > of equals). --> Done. > > Also, can you please test and confirm that the fix works fine for both light > and dark themes on all the platforms (Windows, Mac, Linux)? -> Tested on macOS. (In reply to Matthias Becker from comment #13) > (In reply to Noopur Gupta from comment #9) > > Thanks, Thomas. > > > > Matthias, please update the patch to follow JDT UI coding conventions > > (https://wiki.eclipse.org/JDT_UI/How_to_Contribute#Coding_Conventions). For > > example, we use the compact assignment form (a= b, i.e. no space on the left > > of equals). > --> Done. > > > > Also, can you please test and confirm that the fix works fine for both light > > and dark themes on all the platforms (Windows, Mac, Linux)? > -> Tested on macOS. And tested on windows. can pls. somebody else test on linux. (In reply to Matthias Becker from comment #14) > can pls. somebody else test on linux. Looks fine on Linux (dark and white). Gerrit change https://git.eclipse.org/r/127942 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=0c40c1089b8338ffe690c09c4be80401d9ddda92 Thanks for the contribution, Matthias, Thomas and Lars!! Please verify in the next I build on Windows, Linux and MacOS. Do we need a N&N entry ? (In reply to Sarika Sinha from comment #17) > Thanks for the contribution, Matthias, Thomas and Lars!! > > Please verify in the next I build on Windows, Linux and MacOS. > > Do we need a N&N entry ? Would be nice. But unfortunatelly I don't have the time for it. I'm in the office the last day today and then I am on vacation for 4 weeks. Sorry. This is basically a bug fix. New or massively improved functionality should have a N&N, but simple bug fixes like this don't need one in my opinion. (In reply to Thomas Wolf from comment #19) > This is basically a bug fix. New or massively improved functionality should > have a N&N, but simple bug fixes like this don't need one in my opinion. +1, I don't think this is N&N worthy. |
Created attachment 275506 [details] screen shot showing the issue The last page of the "Externalize String" wizard shows a summary of all changes to be performed. When new files are created here the preview of the new file has light background in the dark theme.