Dragan, the demo site you've put together looks very nice indeed, I strongly encourage other to take a look as well! Comments below.
On Wed, Jun 29, 2011 at 2:47 PM, [email protected] <[email protected]> wrote: > 1. Dropdown menu (see demo page) > - As this is the most common scenario (IMO) fro using a contextmenu, I > tried to provide a dropdown menu that is a separate component meant to be > used as a context menu. This is still Work in Progress and any feedback on > it would be helpfull. > - The goal with the dropdown menu is to make it's usage as simple as > possible making 2nd level submenus in tml etc. Please see the source code > and tml of the demo page (links to the source code are on the demo page > also). Good work, this component (and the underlying context menu) looks relatively close to being ready for inclusion. It might be a good idea to focus on making it complete enough to be included as a patch. There's always room to improve, but it can be enhanced even while it's already part of Tapestry core. That would get you to work more using Apache/Tapestry workflow. Before making a patch though, a few specific issues: - documentation, especially javadoc - the default css should be an asset, rather than pulled from the context - consider using .t-dropdown etc. as CSS style class name instead of .dropdown and so on - too generic name may collide more easily and .t-* is inline with tapestry conventions - copyrights - have you copied any code/styles from anywhere you haven't written yourself? - hideType=mouseOut is always problematic as it depends on where the new object is rendered. At least the current example didn't quite work for me (on FF5, it was hidden as soon as it was rendered) - maybe consider a larger/transparent area around the actual menu as "inside" - what's the wait for in onContextMenuFromGrid1(...) of the dropdown example? > 2. Grid Enhancements - Making pagination and Sorting > Bookmarkable (see demo page) > - This is achieved by modifying the existing GridColumns and GridPager (see > line 141) components with just a few lines of code, combined with mixins for > the actual URL manipulation (bookmarking). > - The main issue why I had to modify GridPager and GridColumns is because > they don't keep (off course) the URL parameters (request parameters) for the > links for the sort columns (in GridColumns) and for the links in the page > numbers (in Grid Pager). The added LOC are just to keep those parameters for > their action and sort events. This approach obviously needs rethinking. One > solution I could think of is decorating the links but this happens in pages > not components, so I would have to add advice to all pages (tried that also > and it worked), but it seems like just too much overhead. > - Another idea is to just make a redirect on setupRender on the grid's > mixins to put the parameters in the url. This is the simplest solution and > it would also work if the grid is sorted in code (not by clicking the > columns), but a redirect is another request. IDK if this can lead to usage > problems. Advice please :) > - So. All Work is done in the 2 mixins ColumnsSort.java (for Grid > Columns) and CurrentPageURL.java (for GridPager) that catch the sort and > action events and add parameters and also read parameters from the Request > to adjust column sorting and pagination. I supose this is quite ok. But the > main issue remains. > "How to keep the request parameters for the links produced by GridPager and > GridColumns?" Unfortunately, I don't have good suggestions for this at the moment. If it was possible to advice the component, it sounds like that could have been a good enough solution but I agree advising all pages is too heavyweight as a solution. For redirects, while it's reasonable safe to assume that the datamodel is backed by some cache, we cannot be sure and so might have an adverse effect on performance. I'll spend a bit more time on it to see if there are other reasonable alternatives. > 3. Expose the cell content in GridCell.java > - Another issue I was advised to look into by my mentor (Kalle) is to > replace the advice that makes ContextMenu.java work for the tapestry grid. > Basically this is the idea: Modify GridCell.java to expose the grid cell > contents (row value and property value not just property value like in > PropertyOutputContext.java) in an environmental. This can be used in far > more scenarios, not just contextmenu for the grid (think of a mixin that > selects a grid row (or cell) and notifies the server via ajax, so you can do > master-detail pages etc). > Replacing the GridCell advice with a few lines of code is next on my work > log so we'll se how it goes. Yes, this makes sense to me and it's fairly cheap to do. Kalle > On Wed, Jun 15, 2011 at 1:27 PM, [email protected] > <[email protected]> wrote: >> >> Hi, >> I have updated the contextmenu with other client events like mousedown, >> mouseover etc. that you can look at on the new examples >> page http://dragansah.com/contextmenu/parametersexamples. >> You can browse the issues (mainly tasks and bugs) >> here http://code.google.com/a/apache-extras.org/p/right-click-menu-gsoc2011/issues/list and >> if someone has some ideas or wishes, they can add them to the issues list. >> Next on my work-log is to provide a dropdown menu as a separate component >> that can be used with the existing contextmenu. It is not directly related >> to the contextmenu because you can put anything in the contextmenu, but >> maybe 90% of the use-cases of a contextmenu is to have a dropdown wiht icon >> and a label like they have on the rich faces demo site under >> RichMenu->ContextMenu. >> Any kind of comments or remarks are very much welcomed. >> Cheers, >> Dragan Sahpaski >> >> >> On Tue, May 31, 2011 at 7:16 PM, Kalle Korhonen >> <[email protected]> wrote: >>> >>> I'm officially Dragan's mentor but the rest of committers should >>> really take a look at Dragan's excellent work if you can just spare a >>> few cycles for it. The demo site (http://dragansah.com/contextmenu/) >>> Dragan put together makes it easy to track the project status even if >>> you don't have time to dive into the code. We already talked about the >>> items via Skype but a few quick comments below.. >>> >>> On Sun, May 29, 2011 at 1:51 PM, [email protected] >>> <[email protected]> wrote: >>> > 1. Two mixins: ContextMenu and ContextMenuAjax or merge them in one >>> > mixin? >>> >>> It doesn't cost any more to use separate mixins for the user so >>> probably makes sense to keep as is for now. >>> >>> > 2. Is the concept of advising GridCell (actualy >>> >>> AbstractPropertyOutput<http://tapestry.apache.org/tapestry5/apidocs/org/apache/tapestry5/corelib/base/AbstractPropertyOutput.html#renderPropertyValue(org.apache.tapestry5.MarkupWriter, >>> > java.lang.String)>) ok? Its clear that we cannot implement support of >>> > the >>> > t5 grid without an advice or I'm I wrong? >>> >>> That's the way and it didn't seem too bad at all, but if it'd be >>> possible to implement it better by making a small, generic and >>> backwards compatible enhancement to GridCell, I think that'd be >>> preferable in the long run, just because there's some performance >>> penalty whenever using an advice, and an AbstractPropertyOutput gets >>> used a lot. >>> >>> > 5. The contextmenu is rendered after the component and there is a bit >>> > of >>> > The quirk is that the contextmenu forces the render of the conainer's >>> > clientId (if it is a t5 ClientElement), and uses it to set a >>> > javascript >>> > event oncontextmenu on it using the id. If the component is not a t5 >>> > ClientElement (t5 Label for example), than this javascript is applied >>> > for >>> > getting the client element in js: $('contextMenuId').previousSibling. >>> > This >>> > code assumes that the parent component is only one html element (div >>> > or >>> > similar), but this is not the case for example the t5 TextField which >>> > renders a trailing icon. Luckily the TextField is covered because it >>> > is a >>> >>> Seems to me it's quite fine to require ClientElement in that case. >>> >>> Kalle >>> >>> --------------------------------------------------------------------- >>> To unsubscribe, e-mail: [email protected] >>> For additional commands, e-mail: [email protected] >>> >> > > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
