Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 370336 - Cell editors shifted when image is used in the item
Summary: Cell editors shifted when image is used in the item
Status: RESOLVED FIXED
Alias: None
Product: RAP
Classification: RT
Component: JFace (show other bugs)
Version: 1.5   Edit
Hardware: All All
: P2 normal (vote)
Target Milestone: 1.5 M6   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-01 10:19 EST by Ivan Furnadjiev CLA
Modified: 2012-02-10 12:16 EST (History)
2 users (show)

See Also:

Attachments
Screenshot from Examples Demo (6.62 KB, image/png)
2012-02-01 10:19 EST, Ivan Furnadjiev CLA 		no flags Details
Tree-Cell spacing fix (606 bytes, patch)
2012-02-10 08:58 EST, Nick Mussin CLA 		no flags Details | Diff
Tree-Cell spacing fix including fixed Tree.test.css (1.28 KB, application/octet-stream)
2012-02-10 10:13 EST, Nick Mussin CLA 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ivan Furnadjiev CLA 2012-02-01 10:19:45 EST 
Created attachment 210377 [details]
Screenshot from Examples Demo

Steps to reproduce:
1. Start Examples Demo
2. Go to TreeViewer page
3. Double-click on an item (first column) to activate the cell editor. Cell editor is shifted to the right (5-6px) and the cell content shines below.
If you remove the item image cell editor is positioned correctly.
Comment 1 Ivan Furnadjiev CLA 2012-02-09 05:12:45 EST 
When I investigated this, two questions came to my mind:
1. Why Tree-Cell and Table-Cell spacing and padding differ in the default theme? Nick, is there a reason not to make them equals (like in Table-Cell for example)?
2. There is a comment in Tree#getImageOffset that left padding is ignored for the tree column. Tim, any specific reason for this? Removing the check for isTreeColumn solves the problem.
Comment 2 Nick Mussin CLA 2012-02-10 08:58:00 EST 
Created attachment 210854 [details]
Tree-Cell spacing fix
Comment 3 Nick Mussin CLA 2012-02-10 08:58:15 EST 
Hi Ivan, no actually it was not intended to have different spacing in Tree-Cell and Table-Cell. I have attached a patch that fixes the theme.
Comment 4 Ivan Furnadjiev CLA 2012-02-10 09:53:20 EST 
(In reply to comment #2)
> Created attachment 210854 [details]
> Tree-Cell spacing fix
Probably you have to fix the theming test (Tree.test.css) as well ;-)
Comment 5 Nick Mussin CLA 2012-02-10 10:09:50 EST 
oh...of course :)
Comment 6 Nick Mussin CLA 2012-02-10 10:13:01 EST 
Created attachment 210855 [details]
Tree-Cell spacing fix including fixed Tree.test.css
Comment 7 Ivan Furnadjiev CLA 2012-02-10 10:39:18 EST 
Nick, thanks for that patch. As this patch is masking the actual problem (waiting Tim to gives his opinion on comment#1 point 2 - real solution) I will apply it afterwards.
Comment 8 Tim Buschtoens CLA 2012-02-10 12:15:26 EST 
The missing padding was implemented because thats the way the classic tree theming needed it to look right. Classic Theme is no longer supported (?), but the arrow icons used in current theme "include" the padding already, so would need to be adjusted when this behavior is changed.
Comment 9 Ivan Furnadjiev CLA 2012-02-10 12:16:45 EST 
After discussion with Tim we agreed to keep the ignorance of left padding for tree column in Tree#getImageOffset (used to position the image on the client). To align with it, TreeItem#getImageBounds adjusted to ignore the left padding for tree column too. With this change cell editors are position correctly. Applied Nick's patch too.