Community
Participate
Working Groups
Currently sapphire supports adding actions to particular contexts, some of which may be in the toolbar area of a section or a form editor page. It would be nice if those toolbar items that represent actions could have additional rendering hints that could change the style of the toolbar item. Namely, image, image+label, or maybe just label-only, etc.
Thanks for the offer to work on a patch for this enhancement. Here are the details on what needs to be done... 1. ISapphireActionDef : This needs a list of hints. Re-use ISapphireHint. Copy hints property definition from ISapphirePart. Ignore the extra accessor methods. 2. Add constants for hint name and values to ISapphireActionDef like in ISapphirePropertyEditorDef. 3. Edit ISapphireHint possible value declarations to support the new hint. 4. In SapphireAction's init method read the hints from the definition into a local map. Add API to SapphireAction to read hints. 5. In SapphireToolBarActionPresentation and SapphireToolBarManagerActionPresentation, read the hint and set the appropriate SWT style bits. 6. Edit SapphireEditorCommon.sdef in org.eclipse.sapphire.sdk plugin to surface action hints in the sdef editor. If you get stuck, don't hesitate to ask.
I've got this working locally but I need to discuss a few decisions before posting the patch to help explain the code. All of your help got me exactly where I need as far as defining the hints and pulling them out of SapphireAction when needed. I also was able to get the hints showing in the sdef editor which is pretty cool the dog fooding that is going on.
Created attachment 193955 [details] Patch attempt #1 In this patch the place I ran into trouble was the actual toolbar rendering for the global Form Page Heading toolbar area. It turns out that the type of Toolbar that is created by the FormHeading class does not have the appropriate style bits to allow for image + label on an action where the label could be displayed on the right. Originally I was able to get it to work using a "FORCE_TEXT" mode for an action contrib item but it displayed the text below the image and made the toolbar change size to be way to tall and for images that didn't have the image+label hint it looks extremely odd. My original desire was to be able to put the text to the right of the image anyways and that is consistent with other parts of eclipse (perspective bar, etc). For the toolbars that are on individual form sections, we create that toolbar and I was able to pass in the correct style bit (SWT.RIGHT) see SapphireSection.java line 111 (after patched) So if FormHeading.java line 764 had variable style bits and we could some how pass in SWT.RIGHT as another style bit, things would be much better. But since that isn't possible the only way I could get the rendering to look correct was a bit of a hack. In SapphireToolBarManagerActionPresentation instead of putting user actions directly on the toolbarmanager from FormHeading, I created a subclass of ControlContribution so I could put my own control on there, namely another toolbar. This toolbar I provide the correct style bits to allow for nice image+label to work as expected.
Also in my patch I tried to add an example of a action added to the toolbar of the sapphire-gallery.xml editor only. This action should have image+label style. I'm not sure if it is showing up correctly since on my system I can't seem to test the samples plugin from PDE when I launch a runtime workbench. It says "can't find IGallery" like the annotations processor isn't correctly generated the classes. I'll attach a screenshot of the patch working on my system.
Created attachment 193956 [details] sapphire_image+label.png Here is an action on the sapphire.editor toolbar that has image+label hint
Very nice. Just a bit of cleanup and this patch should be ready to be released... 1. Headers need to be added/updated on all new/updated files. For new files copy headers from other files and update accordingly. Each file you are add should list your employer as copyright holder and you as the maintainer in the contributors section. In the class javadoc, @author tag is required. It should be the same individual as listed as the maintainer in the header. Each file you update, you need to add a line under contributors in the header, listing your name and the bug. See SapphireActionSystemPart for the pattern to follow. 2. I like that you added an example action, but it would be more appropriate to declare this action and handler locally in the sdef file (under page definition). I notice that sdef editor lacks the UI for that. I will fix that, but the syntax is the same as in the extensions file, except that you do not typically need to supply a condition (the benefit of the local context). 3. I am fine with the ControlContribution workaround, but if we are going to go that route, it should be possible to delegate most of the behavior to SapphireToolBarActionPresentation, so that we collapse those two code paths into one as much as possible. Maybe SapphireToolBarManagerActionPresentation just becomes a thin wrapper... 4. In SapphireToolBarManagerActionPresentation.SapphireToolbarContribution, there are few warnings... Please prefix access to items member with "this." (convention in the Sapphire codebase). The line "embeddedToolbar.setCursor(FormsResources.getHandCursor());" should go as we should match the style of toolbars elsewhere in eclipse and not use the hand cursor. > Also in my patch I tried to add an example of a action added to the toolbar of > the sapphire-gallery.xml editor only. This action should have image+label > style. I'm not sure if it is showing up correctly since on my system I can't > seem to test the samples plugin from PDE when I launch a runtime workbench. > It says "can't find IGallery" like the annotations processor isn't correctly > generated the classes. I'll attach a screenshot of the patch working on my > system. The sample worked for me. I have definitely experienced the behavior that you are describing. As far as I been able to figure it out, it a weird bug in Eclipse JDT annotation processor support. In certain circumstances (that I have not been able to identify), the annotation processor just isn't getting invoked on all the classes. I've yet to pin it down to something that I can file as a meaningful bug on JDT. It doesn't seem to be a function of specific code pattern. It does seem to be a function of workspace state and the number of projects that require annotation processing that are open. I have been able to workaround some occurrences of this problem by closing some projects that I wasn't working on at the moment.
A few more items... 1. I have added "style" hint for an unrelated item. It is defined in ISapphirePartDef. Let's re-use that name here instead of "action.style". The possible values for style under different parts should vary as appropriate. 2. Let's call the three styles: "image", "text" and "image+text". 3. Take a look at SapphireWithDirective for my preferred method of handling the style hint. Could you mimic that pattern here?
Oh and one more thing... We will need to add a blurb about this feature in the 0.3 release what's new document. That's located in the doc plugin under html/releases/0.3/index.html.
(In reply to comment #7) > A few more items... > > 1. I have added "style" hint for an unrelated item. It is defined in > ISapphirePartDef. Let's re-use that name here instead of "action.style". The > possible values for style under different parts should vary as appropriate. > > 2. Let's call the three styles: "image", "text" and "image+text". > > 3. Take a look at SapphireWithDirective for my preferred method of handling the > style hint. Could you mimic that pattern here? By reusing to you mean to re-declare a HINT_STYLE ="style" in ISapphireActionDef or to just reference the existing ISapphirePartDef.HINT_STYLE in SapphireAction classes?
> By reusing to you mean to re-declare a HINT_STYLE ="style" in > ISapphireActionDef or to just reference the existing > ISapphirePartDef.HINT_STYLE in SapphireAction classes? No need to redefine. Just reference it from ISapphirePartDef.
Created attachment 194106 [details] Patch update #2 I've made all the suggested changes, minus the html in the 0.3 releases folder. I'll work on that next but I wanted to get some feedback on the changes in the meantime.
Created attachment 194109 [details] Patch update #3 Added another patch that fixed a type in an example and also now provides the HTML content for the 0.3 release.
Looks good. Patch released after a few tweaks.
Verified after installing build#300
Closing.
