[Yanel-dev] Patch: Make toolbar replaceable

[Michael Wechner]

Hi Simon

Your ideas sound great. Please turn them into patches ;-)

I am confident that we can make the way how we integrate the toolbar 
even more configurable,
such that we can give people the choice how they want to do it (like now 
or with a hook or whatever)
and that by "evolution" a best practice will "materialize".

Thanks

Michael

On 11/8/10 3:24 PM, simon wrote:
> hi all
>
> thanks for the patch. last week i played around with the toolbar and 
> had some ideas:
>
> - instead of inserting the toolbar structure into every html page. 
> inventing a menu resource-type which would provide the menu structur. 
> this way the structure could be used by an ajax menu which would 
> probably be faster.
> - yanel only inserting one javascript file, one css file and one 
> hook-element into a html page. get rid of the inline style used by the 
> toolbar.
> - i think there are better menu scripts available [1] which are 
> crossbrowser compatible without the quirks we use today for IE6
> - the menu should delay the disappearance after the mouse leaves the 
> menu. at the moment i's hard to navigate if the menu has many levels.
> - a contract between menu and resource-types so that resource-types 
> can add menu items. resp the menu asks the current resource-type for 
> menu items. e.g. Menuable
>
> [1]http://www.dynamicdrive.com/dynamicindex1/ddsmoothmenu.htm
> http://pupunzi.com/#mb.components/mb._menu/menu.html
>
>
> simon
>
> Am 08.11.2010 11:08, schrieb Michael Wechner:
>> Hi Rob
>>
>> On 11/5/10 3:50 PM, Rob Adamson wrote:
>>> Hi Michael,
>>>
>>> Thanks for reviewing. I've made some changes, and here's a new patch.
>>
>> thanks very much
>>> Note that I want to check I can implement our new toolbar with this,
>>> so please review it but maybe don't commit it just yet.
>>
>> I have commited it now and added a sample custom toolbar (based on 
>> your DefaultToolbar)
>>
>> src/realms/from-scratch-realm-template/src/java/org/wyona/yanel/servlet/toolbar/impl/FromScratchRealmToolbar.java 
>>
>>
>> For the moment we just don't announce this officially, hence my 
>> WARNING to everybody please
>> use this for the moment at your own risk ;-)
>>
>>
>>> Note that one thing I've done in the DefaultYanelToolbar is to catch
>>> non-RuntimeExceptions and wrap them in YanelToolbarExceptions.
>>> Throwing and catching Exception is a rather bad idea, in general, as
>>> it is the worst of both worlds (checked&  unchecked) - the programmer
>>> gets no extra information as to what may be thrown, but still has to
>>> catch all Exceptions or declare that they may be thrown.
>>
>> but do you log the original exceptions?
>>
>>
>>> Anyway, more comments inline below...
>>>
>>> On 5 November 2010 08:57, Michael 
>>> Wechner<michael.wechner at wyona.com>  wrote:
>>>> On 11/4/10 7:28 PM, Rob Adamson wrote:
>>>>> It re-uses the<menu>    entry in the realm configuration - if the 
>>>>> class
>>>>> implements the new interface YanelToolbar, this is used, otherwise 
>>>>> the
>>>>> DefaultYanelToolbar is used and the<menu>    entry is treated as a
>>>>> subclass of org.wyona.yanel.servlet.menu.Menu.
>>>>
>>>> I guess this is done within
>>>>
>>>> src/webapp/src/java/org/wyona/yanel/servlet/YanelHTMLUI#getYanelToolbar(Resource 
>>>>
>>>> resource)
>>>>
>>>> right?
>>>>
>>>> Maybe it would make sense to introduce a new tag called<toolbar/>, 
>>>> but we
>>>> can always do that later.
>>> I did think of that, but changing the Realm interface would break
>>> compatibility :)
>>> Hence overriding<menu>.
>>
>> agreed. As soon as we have consolidated your enhancements and got 
>> some more experience with
>> it we can explain this within the documentation
>>
>>
>>>>> I had to give YanelServlet.getIdentity public visibility  (previously
>>>>> it was package visibility).
>>> I've done away with this change. Calling
>>> resource.getRealm().getEnvironment().getIdentity() seems to do the job
>>> just as well.
>>>
>>>> I have three requests in order to commit the patch more easily:
>>>>
>>>> 1) Please change the formatting: 4 spaces instead 1 tab
>>> OK
>>>
>>>> 2) Please add some javadoc to the new methods
>>> OK
>>>
>>
>>
>> thanks very much
>>>> 3) Please create a simple minimal toolbar custom implementation for 
>>>> the
>>>> from-scratch-realm which we could
>>>> use as example:
>>>> https://svn.wyona.com/repos/public/yanel/trunk/src/realms/from-scratch-realm-template/src/java 
>>>>
>>>>
>>>> to show the difference between the Default implementation and a custom
>>>> implementation
>>> I haven't got time to do this. You will see the implementation in my
>>> realm, and perhaps you/I could create an example implementation from
>>> that at a later date.
>>
>> I have done a "lazy" version to at least test the loading
>>
>> src/realms/from-scratch-realm-template/src/java/org/wyona/yanel/servlet/toolbar/impl/FromScratchRealmToolbar.java 
>>
>>
>> but need to make it a bit more different, such that one can actually 
>> see a difference.
>>
>> Simon has told me off the list that he did some improvements recently 
>> and I will ask
>> him to implement these within this implementation.
>>
>> Thanks again for your patch and please continue :-)
>>
>> Michael
>>> Kind regards,
>>>
>>> Rob
>>
>