Bugzilla – Full Text Bug Listing
| Summary: | [win32] Numbers in Spinner control are not vertically centred | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Phil Beauvoir <p.beauvoir> | ||||||||
| Component: | SWT | Assignee: | Alexandr Miloslavskiy <alexandr.miloslavskiy> | ||||||||
| Status: | VERIFIED FIXED | QA Contact: | Niraj Modi <niraj.modi> | ||||||||
| Severity: | normal | ||||||||||
| Priority: | P3 | CC: | alexandr.miloslavskiy, loskutov, lshanmug, niraj.modi | ||||||||
| Version: | 4.17 | Keywords: | regression | ||||||||
| Target Milestone: | 4.18 RC1 | Flags: | lshanmug:
review+
|
||||||||
| Hardware: | PC | ||||||||||
| OS: | Windows 10 | ||||||||||
| See Also: |
https://bugs.eclipse.org/bugs/show_bug.cgi?id=565679 https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/172482 https://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=91c971d35f1e2f4c79104f50feca0cbfa6a4f708 |
||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Phil Beauvoir
Created attachment 284697 [details] Screenshot comparing two versions of Spinner control Running Snippet184 I took some screenshots comparing the Spinner in Eclipse 4.15 and 4.18. https://git.eclipse.org/c/platform/eclipse.platform.swt.git/plain/examples/org.eclipse.swt.snippets/src/org/eclipse/swt/snippets/Snippet184.java What about 4.16? Is this OK or also broken? (In reply to Andrey Loskutov from comment #2) > What about 4.16? Is this OK or also broken? No, that version is OK. Could it be this change? https://github.com/eclipse/eclipse.platform.swt/commit/ddd5919b91cfc9f0cfb4eef65d19333e495373d1#diff-f033e4b8a392b1032f2640db2b9ffd62290c030ad111a8f31c5047587d764b62 (In reply to Phil Beauvoir from comment #4) > Could it be this change? > > https://github.com/eclipse/eclipse.platform.swt/commit/ > ddd5919b91cfc9f0cfb4eef65d19333e495373d1#diff- > f033e4b8a392b1032f2640db2b9ffd62290c030ad111a8f31c5047587d764b62 Yes, it is. If I revert commit https://github.com/eclipse/eclipse.platform.swt/commit/ddd5919b91cfc9f0cfb4eef65d19333e495373d1 I see the old centered text, so this is a regression from bug 565679 changes. @Alexandr, could you please take a look? Created attachment 284736 [details]
Text vs Spinner
In old code, the text's position was in fact a bug caused by border. I fixed that bug.
However, I do agree that it looked better before. It's a kind of a bug that eventually is seen as intentional design, I guess. Also, I have now compared Text to Spinner, and Text's text position is slightly more centered.
The fix should be very simple: Spinner's internal Text needs to be moved a couple pixels right-down.
I'll have a look later. If anyone is willing to do it for me, you're welcome.
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/172482 Created attachment 284811 [details]
Test snippet's screenshot with patch
I have adjusted Spinner's text position to match Text's position. This results in text being moved 2px lower. While this is still not centered, well this is how SWT's Text has been for years. So, I owe you a beer then, Alex. :-) I'll test when it gets into an I-build. Cheers! (In reply to Eclipse Genie from comment #7) > New Gerrit change created: > https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/172482 Change looks good to me. Fix is localized for Spinner's size adjustment. Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/172482 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=91c971d35f1e2f4c79104f50feca0cbfa6a4f708 (In reply to Eclipse Genie from comment #12) > Gerrit change > https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/172482 was > merged to [master]. > Commit: > http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/ > ?id=91c971d35f1e2f4c79104f50feca0cbfa6a4f708 Thanks Alexandr for the fix, resolving now. (In reply to Phil Beauvoir from comment #10) > So, I owe you a beer then, Alex. :-) > > I'll test when it gets into an I-build. > > Cheers! Tested on I20201120-1800, looks good. Thanks! (In reply to Phil Beauvoir from comment #14) > (In reply to Phil Beauvoir from comment #10) > > So, I owe you a beer then, Alex. :-) > > > > I'll test when it gets into an I-build. > > > > Cheers! > > Tested on I20201120-1800, looks good. Thanks! Thanks Phil for the verification & confirmation. |