Bugzilla – Full Text Bug Listing
| Summary: | Tree Widget does not support PaintItem events | ||
|---|---|---|---|
| Product: | [RT] RAP | Reporter: | Jürgen Becker <juergen.becker> |
| Component: | RWT | Assignee: | Project Inbox <rap-inbox> |
| Status: | NEW --- | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P3 | CC: | christian.campo, elias, ivan, rsternberg, steffen.kriese, tbuschto, tonny.madsen |
| Version: | 1.4 | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | All | ||
| Whiteboard: | |||
| Bug Depends on: | |||
| Bug Blocks: | 302893 | ||
| Attachments: | |||
Hi Jürgen, thanks for the patch. Could you cleanup your patch a little bit (blank characters, code formatting...) to contain only changes related to the issue? This will help as to evaluate it easily. Support for attribute "pointer-events" only exists in a few browser. Sorry, for the mangled patch. I formatted the code with the Formatter defined in the project settings. Are they not up to date? Most of the unnecessary patched lines result from reformatted javadoc. The formatter definition forces a line wrap after 80 chars. The code from the repository is not formatted that way. Where can I get a formatter definition that fits? Regarding the css style "pointer-events" I will investigate a little bit more. Thanks Tim for pointing that out. Jürgen, don't format the existing code at all. Just put your code in there. As I understand Juergen you have a formatter defined in your project file, but you dont use it ? Why is that again ? I agree its hard to evaluate the patch now that it contains all the formatting issues that are not part of the new code. But its equally hard to now sort out what is the formatting delta and what is the added code in the patch. In Riena i.e. we force format on save on every class that is in a Riena bundle. We also eventually developed a plugin that reformats every bundle in the workspace (only to find out that its a JDT functionality) You probably wouldnt consider to resave the affected classes with your favorite reformatter and store the result in CVS so that Juergen could recreate the patch for there ? We don't use formatters (save actions) as there are some original SWT/RCP code in RAP bundles that we don't want to format. Thus, it's easy to compare and adopt the updated code from SWT/Platform. But you are right - there is a formatter defined in our org.eclipse.rap.rwt and org.eclipse.rap.rwt.q07 bundles. Ralf, Ruediger, can we remove it? What do you think? Created attachment 189797 [details]
Cleaned Tree PaintItem event Workspace patch for org.elcipse.rap.rwt and org.eclipse.rap.rwt.q07
I cleaned up the patch. It would have been a bit easier if the formatting of the code from the repository would match the formatting which is defined in your project settings.
The css style "pointer-events" is currently only supported by the modern browsers. Meaning crappy IE does not support it and who knows if it ever will. :( I try to find another solution for the mouse event problem. I played with our Riena application running on patched RAP and it is working fine (mostly). At least with IE 8. I try to get my hands on some older IE versions. As far as I could find out it boils down to this: The IE ignores areas of the canvas where nothing was painted and propagates the events like the canvas with style "pointer-events: none". The only problem remaining is that the areas where we draw anything will not propagate the mouse events. Hi Jürgen, thanks a lot for this patch. Apart from some details that could easily be fixed, I see two major problems here: 1) The event propagation from the canvas overlay to the Tree itself. As it has been pointed out already, the pointer-events solution is not supported by all browsers. We came across this problem in different areas already (e.g. when implementing shadows using vector graphics), and didn't find a reliable cross-browser solution. Please understand that as a framework that offers cross-browser compatibility, we cannot accept a solution that affects working functionality on some browsers. 2) Scrolling, expanding, collapsing. As far as I can see, your patch does not support scrolling. When the user scrolls the Tree, the canvas has to be scrolled as well. The region that has been scrolled into view would have to be painted upon and the region scrolled out of view would have to be discarded. The same applies to expanding and collapsing of Tree items. Generally, I think an already painted drawing should never cover the wrong Tree item just because the item metrics changed because of scrolling, expanding or collapsing. I believe that the patch works well for you and it may also help others, but I think that we can only make this functionality available in RAP when it covers all browsers and all Tree use cases. If you like to continue working on this functionality, we will be happy to support you! (In reply to comment #5) > As I understand Juergen you have a formatter defined in your project file, but > you dont use it ? Why is that again ? We use it, but not to blindly format the entire file. I just updated our coding conventions wiki page to explain how the formatter should be used for RAP: http://wiki.eclipse.org/RAP/CodingConventions > In Riena i.e. we force format on save on every class that is in a Riena bundle. > We also eventually developed a plugin that reformats every bundle in the > workspace (only to find out that its a JDT functionality) We can't do this in RAP, firstly because the formatter does not cover all of the coding conventions we have and we believe in. Even though auto-formatting code is a nice feature, we try to craft our code carefully and have some strong opinions on how it should look like. The second reason is that we have a lot of verbatim copies from SWT and RCP that must not be re-formatted. |
Created attachment 189783 [details] Tree PaintItem event Workspace patch for org.elcipse.rap.rwt and org.eclipse.rap.rwt.q07 For Riena on RAP we need the PaintItem events for the Tree widget to display some markers in the module navigation tree. I added the PaintItem event handling to the Tree widget. The event is fired from TreeLCA.doRedrawFake(...). To make the markers work I also had to add a GC to the Tree widget (see Tree.getAdapter(...). On the JavaScript side the GC needed some tweaking. Normally the canvas created by the GC would be behind the Tree, so I changed the z-index style property (only for the Tree GC/canvas). This results in another problem: all mouse events are swallowed by the canvas layer, which now lays over the Tree divs. Clicks or hovers to not work any longer. Luckily there is an easy fix by setting the style property "pointer-events" to "none". I attached a workspace patch for the changes I made. Maybe some of the RAP developers can give me feedback for my solution. If I can improve it or if you have any questions, please let me know.