ItemClickHandler did not work (would fetch() kill the handler?)

[leeyuiwah]

Hi,

I have a piece of code that look very similar to the example code in http://www.smartclient.com/smartgwt/...ry_treebinding

It has a menu and I set the menu to have a ItemClickHandler by setItemClickHandler(). There is, however, a difference -- the menu items in my code are populated dynamically -- when I push to the menu, it would issue a fetch with a certain Criteria (I could see the RPC flied by).

Here are some snippet of the code.

Would the filterData() destory an ItemClickHandler that was previously set to a Menu?

private ItemClickHandler dexiClickHandler = new ItemClickHandler() {

    @Override
    public void onItemClick(ItemClickEvent event) {
            
        MenuItem item = event.getItem();
        SC.say("Item clicked" + item.getAttributeAsString("id"));
    }
};

...

Criteria criteria = new Criteria();
criteria.addCriteria("wbId", wbId);

DataSource dataSource = DataSource.get("naraDexiDS");

recentDepSubMenu.setInitialCriteria(criteria);

recentDepSubMenu.setCriteria(criteria);
recentDepSubMenu.setDataSource(dataSource);
recentDepSubMenu.addItemClickHandler(dexiClickHandler);

/*
 * The next two lines are key -- even though a fetchData() may have been
 * issued when the menu is first drawn, a subsequent user-interaction
 * of pushing to the menu will trigger another filterData() that honors
 * the Criteria
 */
recentDepSubMenu.setAutoFetchData(true);        // issue a fetch/filter upon first drawing
recentDepSubMenu.setAutoFetchTextMatchStyle(TextMatchStyle.EXACT);

Version: SmartClient Version: v10.0p_2015-06-28/PowerEdition Deployment (built 2015-06-28)
Browser: Tried on Firefox (26.0) and IE (9.0.8112.16421)

[Isomorphic]

No, fetching data from the server would not normally destroy an ItemClickHandler that was added to the Menu as a whole (it would of course destroy anything set on an items, since they would be completely replaced).

The part of your code that you've shown looks fine. Let us know if you can put together code that we can run to see the problem.

[leeyuiwah]

Hi,

My code does seems to suggest it was the FETCH RPC that kills the ItemClickHandler. Specifically

1. if setAutoFetchData(true), a second fetch() RPC would be seen
==> ItemClickHandler will be lost

2. if setAutoFetchData(false), no more fetch() RPC would be seen
==> ItemClickHandler will remain active

        /*
         * The next two lines show the problem seems to related to the fetch() RPC:
         * 
         * 1. if setAutoFetchData(true), a second fetch() RPC would be seen
         *    ==> ItemClickHandler will be lost
         *    
         * 2. if setAutoFetchData(false), no more fetch() RPC would be seen
         *    ==> ItemClickHandler will remain active
         */
        recentDepSubMenu.setAutoFetchData(true);        
//        recentDepSubMenu.setAutoFetchData(false);

I am going to email you my stripped down test program file, a ds.xml file, as well as an exmaple data file to your support address. Thanks!

[Isomorphic]

Perhaps too obvious to mention, but you seem to be saying that setting autoFetchData to true causes a duplicate fetch and removes your handler. So what about just not using this setting? For Menus, the fetch is automatic anyway.

[leeyuiwah]

Thanks for the suggestion. I noticed that the first FETCH does not have my Criteria applied (see my code fragment in #1 of this thread). The second FETCH will have the Criteria applied, that is why I enabled autoFetchData().

I have sent the files to your support email address. Thanks!

[Isomorphic]

Right, the fetch you see initiated when you do not set autoFetchData is a special behavior of Menu - it is not the same as the superclass ListGrid's fetch behavior, which can be controlled by autoFetchData, initialCritera, etc, but also by a lot of properties that clearly wouldn't apply to menu (fetchAheadRatio, etc).

If you want your Menu to fetch a certain set of records via criteria, the simplest way to do this is to define an operationBinding that automatically applies criteria.  If the criteria is both client-side and dynamic, use DataSource.transformRequest to inject it.

In the meantime, we'll look at clarifying the docs around Menu's automatic fetching behavior.  We may, in a future version, allow specific properties from ListGrid to influence this fetch in the way you were expecting.

[Isomorphic]

By way of a follow-up - from a quick test, we think you should be able to do this without doing anything fancy.

This is what you should be doing to get a Menu which issues only one fetch that includes your criteria.

1) do not call setAutoFetchData(true) - menus fetch automatically anyway, and it's possible that actions taken in this codepath could affect menu behavior

2) do not call setCriteria(), which is essentially non-functional in menus - but, again, could have side-effects

3) DO still call setInitialCriteria() - this is the correct mechanism, and it's been tested today with success

We've made some documentation changes to clarify this.

Let us know if that sorts out your expected fetching scenario - from there we can consider if there is a bug in the ItemClickHandler, which remains an unlikely situation.

Note that, if you do need to follow up regarding ItemClickHandler, we *will* need to see a minimal and standalone, runnable test case that shows the issue.

[leeyuiwah]

Hi, Thanks for the suggestion. I did all three steps that you recommended, but the code still did not work. What I saw is that only one fetchData() was issued, but it does not include the criteria that I set with setInitialCriteria(). The itemClickHandler was NOT destroyed like in my previous tests.

I need to get the Criteria applied. I am willing to pay for the cost of an extra (second) fetchData() but also need the itemClickHandler to remain active.

I think the test files that I sent you clearly show that with setAutoFetchData(true) or setAutoFetachData(false) would affect whether or not the itemClickHandler be destroyed. This seems like a bug to me.

My code does seems to suggest it was the FETCH RPC that kills the ItemClickHandler. Specifically

1. if setAutoFetchData(true), a second fetch() RPC would be seen
==> ItemClickHandler will be lost

2. if setAutoFetchData(false), no more fetch() RPC would be seen
==> ItemClickHandler will remain active

[Isomorphic]

Hello leeyuiwah,

To reiterate, setAutoFetchData() is not an API that applies to Menu. If you call this API, with either true or false, nothing that happens from then on would be considered a "bug". The same is true of setCriteria().

With setInitialCriteria(), while this is another example of a ListGrid API that does not have any documentation telling you that it applies to fetches caused by Menu's use of DataSource (which is again, completely unlike that of the ListGrid), we are looking at designating this an officially supported API for Menu, and in our testing, it actually works without issues.

So, please revisit your test code, wipe out *all* calls to any ListGrid-inherited APIs other than setInitialCriteria(), and let us know if that criteria is being applied as you hope - in our testing, this is working fine.

[leeyuiwah]

Thanks for your response. In my test, I did not call the setAutoFetchData() method, but overall the code still did not work (setInitialCriteria() did not work as expected). I am going to mail you the test program that I used. Thanks!

[Isomorphic]

Hi leeyuiwah, the best thing with test code that is meant to demonstrate a bug is to post it here. If the forums upload constraints seem to get in your way, that's actually an indication that you have not made the test case sufficiently minimal.

For example, if the bug you are claiming is that setInitialCriteria() just doesn't work with a Menu with a DataSource, that should really only require one, very short Java file (if it's a real bug), since all that is needed is to see that the request lacks the criteria; the response doesn't even matter.

[leeyuiwah]

Hi, I got distracted by some other more urgent matters and went silent for a while. However, this problem has not been solved for me.

I think what I want to achieve should be reasonable:

1. The menu's items are to be dynamically populated -- with a Criteria applied
2. The menu's ItemClickHandler should not be destroyed after a fetch operation (be it auto or manual)
3. If need be, I am willing to re-add the ItemClickHandler (perhaps in a callback function)
4. I can accept your suggestion that setAutoFetchData() should NOT be called -- as long as I can achieve the above three points. If need be, I am okay to use a manual fetchData() -- even if that means I have to provide an extra button [Refresh] to my end users.

Can the above be achieved? By the way, I am using 5.0-p21050628. Do you think I need to upgrade to the latest? (For example, I tried the setInitialCriteria() method, but it did not appear to have any effect (that is, the Criteria specified was not used in the fetch operation)

[Isomorphic]

So again, in our testing, the approach we've been discussing works fine and covers all of your needs. If it's not working for you, yes it makes sense to test with the latest, but if the problem still occurs there, we need a test case that reproduces the issue.

[leeyuiwah]

Hi,

I stripped down my test program and get the following latest result (which is documented in the JavaDoc section of the class) -- setInitialCriteria() now works with Menu but UNFORTUNATELY any ItemClickHandler added to the Menu would be killed by a fetchData(). You have said that we should not use setAutoCatchData() with a Menu. Can we use fetchData() with Menu?

I will email you the other two supporting files for repeating the test.

package com.foo.bar;

import com.google.gwt.core.client.EntryPoint;
import com.google.gwt.event.shared.HandlerRegistration;
import com.smartgwt.client.data.Criteria;
import com.smartgwt.client.data.DSCallback;
import com.smartgwt.client.data.DSRequest;
import com.smartgwt.client.data.DSResponse;
import com.smartgwt.client.data.DataSource;
import com.smartgwt.client.util.SC;
import com.smartgwt.client.widgets.events.ClickEvent;
import com.smartgwt.client.widgets.events.ClickHandler;
import com.smartgwt.client.widgets.menu.Menu;
import com.smartgwt.client.widgets.menu.MenuItem;
import com.smartgwt.client.widgets.menu.events.ItemClickEvent;
import com.smartgwt.client.widgets.menu.events.ItemClickHandler;
import com.smartgwt.client.widgets.toolbar.ToolStrip;
import com.smartgwt.client.widgets.toolbar.ToolStripMenuButton;

/**
 * T4: Use Menu.setInitialCriteria(), and then add a [Refresh] button
 * to trigger a manual refresh.  The refreshClickHandler would also manually
 * add the needed ItemClickHandler again -- after we first did a
 * HandlerRegistration.removeHandler().<p>
 *
 * Results: Criteria successfully applied; ItemClickHandler was working initially.
 * After back-end data change, the "Open" menu did not get the latest entries,  
 * but a click on the [Refresh] button could update the "Open" menu.
 * UNFORTUNATELY, after that, the click handler stopped working.<p>
 *
 * So we added a callback function to add
 * the ItemClickHandler again (we confirmed that the callback
 * function was run) but still we did not have any ItemClickHandler
 * after the refresh.<p>
 *
 * We further tried to be more careful as follows. We remembered the HandlerRegistration
 * (returned in the first addItemClickHandler), and then in our callback
 * function, we used the HandlerRegistion to remove any
 * handlers associated with the Menu before we re-add the ItemClickHandler.  Still
 * we could not have the ItemClickHandler working.
 *
 */
public class T4SetInitCritMfetchMaddItemClickHandlerCarefully implements EntryPoint {

    final private String    winWidth    = "100%";
    final private String    labelData   = "Data";
    final private String    labelOpen   = "Open";
    final private String    labelRefresh = "Refresh";
    
    private static ItemClickHandler
    dexiItemClickHandler = new ItemClickHandler() {

        @Override
        public void onItemClick(ItemClickEvent event) {
            MenuItem item = event.getItem();
            SC.say("dexiItemClickHandler.onItemClick(): "
                    + item.getAttributeAsString("id"));
        }
        
    };
    
    private DSCallback cbAddClickHandler = new DSCallback() {

        @Override
        public void execute(DSResponse dsResponse, Object data,
                DSRequest dsRequest) {
            SC.say("in callback cb_addClickHandler");
            /*
             * carefully remove any old handler
             */
            regDexiIcHandler.removeHandler();
            openSubMenu.addItemClickHandler(dexiItemClickHandler);
        }
    };
    
    private ClickHandler
    refreshClickHandler = new ClickHandler() {

        @Override
        public void onClick(ClickEvent event) {
            openSubMenu.fetchData(criteriaOpenSubMenu, cbAddClickHandler);
        }
        
    };
    
    private Menu        openSubMenu;
    private Criteria    criteriaOpenSubMenu;
    private HandlerRegistration regDexiIcHandler;
    
    @Override
    public void onModuleLoad() {
        ToolStrip toolStrip = new ToolStrip();
        toolStrip.setWidth((winWidth));
        
        Menu        dataMenu = new Menu();
        MenuItem    miOpen  = new MenuItem(labelOpen);
        dataMenu.addItem(miOpen);
        
        openSubMenu = new Menu();
        
        DataSource  dataSource = DataSource.get("naraDexiDS");
        criteriaOpenSubMenu = new Criteria();
        criteriaOpenSubMenu.addCriteria("wbId", 3);
        openSubMenu.setDataSource(dataSource);
        openSubMenu.setInitialCriteria(criteriaOpenSubMenu);
        regDexiIcHandler = openSubMenu.addItemClickHandler(dexiItemClickHandler);
        
        miOpen.setSubmenu(openSubMenu);

        ToolStripMenuButton
            dataButton = new ToolStripMenuButton(labelData, dataMenu);
        toolStrip.addMenuButton(dataButton);
        
        ToolStripMenuButton
            refreshButton = new ToolStripMenuButton(labelRefresh);
        refreshButton.addClickHandler(refreshClickHandler);
        toolStrip.addMenuButton(refreshButton);
        
        toolStrip.draw();
    }

}

[Isomorphic]

As you guessed, the new usage you are showing here is incorrect. fetchData() is a ListGrid API and does not apply to Menu's use of DataSource. Your MenuItemClickHandler isn't removed, but the data structure of the Menu gets corrupted because the superclass API you're calling - ListGrid.fetchData() - tries to set up the data structure of a ListGrid.

We don't know what real-world scenario would require calling fetchData() manually on a Menu. If you are trying to build a Menu based on the latest data from the server, the simplest thing would be to create a new Menu instance, which in the case of rebuilding the entire set of MenuItems, is really no more expensive than re-using the existing Menu.