[FIXED] [#1648] TabCloseMenu plugin - issue with icons and events

  1. #1

    [FIXED] [#1648] TabCloseMenu plugin - issue with icons and events

    Hello support team,
    please take a look at the following tab panel:

    @using Ext.Net;
    @using Ext.Net.MVC;
    
    @{
        ViewBag.Title = "Tab Close Menu";
        Layout = null;
    
        var X = Html.X();
    
        Ext.Net.MVC.MvcResourceManager.RegisterGlobalIcon(Icon.TabDelete);
    }
    
    <!DOCTYPE html>
    
    <html>
    <head>
        <title>Ext.NET MVC Test Case</title>
    </head>
    
    <body>
        @(X.ResourceManager())
    
        @X.DisplayField().ID("version").ReadOnly(true).Margin(10).Width(200)
    
        @(X.TabPanel()
            .Height(100)
            .Items(
                X.Panel()
                    .Title("Tab 1")
                    .Closable(true)
                    .Listeners(l =>
                    {
                        l.BeforeClose.Handler = "console.log('BEFORE_TAB_CLOSE_1', arguments)";
                        l.BeforeRemove.Handler = "console.log('BEFORE_TAB_REMOVE_1', arguments)";
                    }),
    
                X.Panel()
                    .Title("Tab 2")
                    .Closable(true)
                    .Listeners(l =>
                    {
                        l.BeforeClose.Handler = "console.log('BEFORE_TAB_CLOSE_2', arguments)";
                        l.BeforeRemove.Handler = "console.log('BEFORE_TAB_REMOVE_2', arguments)";
                    }),
    
                X.Panel()
                    .Title("Tab 3")
                    .Closable(true)
                    .Listeners(l =>
                    {
                        l.BeforeClose.Handler = "console.log('BEFORE_TAB_CLOSE_3', arguments)";
                        l.BeforeRemove.Handler = "console.log('BEFORE_TAB_REMOVE_3', arguments)";
                    })
            )
            .Listeners(l => {
                l.BeforeTabMenuShow.Handler = "console.log('BEFORE_TABMENU', arguments)";
                l.BeforeTabClose.Handler = "console.log('BEFORE_CLOSE', arguments)";
                l.BeforeRemove.Handler = "console.log('BEFORE_REMOVE', arguments)";
            })
            .Plugins(new TabCloseMenu()
            {
                CloseTabText = "CLOSE",
                CloseTabIcon = Icon.TabDelete,
                CloseOthersTabsText = "CLOSE OTHER",
                CloseAllTabsText = "CLOSE ALL"
            })
        )
    
    </body>
    </html>
    
    <script type="text/javascript">
        Ext.onReady(function () {
            Ext.getCmp("version").setValue("Ext JS " + Ext.getVersion().version + " / " + "Ext.NET " + Ext.net.Version);
        });
    </script>
    I'm faced to following issues:

    1. While CloseTabText works as expected, the CloseTabIcon setting does nothing.

    2. I tried to solve this in BeforeTabMenuShow.Handler, but the event does not fire.

    3. Then I tried to solve it by override:
    Ext.define("Nelis.TabCloseMenu", {
        override: "Ext.ux.TabCloseMenu",
    
        onContextMenu: function (event, target) {
            this.callParent(arguments);
            this.menu.child("#close").setIconCls("icon-tabdelete");
        }
    });
    ... but it also does not work.

    4. Finally, I'm quite confused about triggering events. To validate applications within our framework, we listen to the Close event that is fired by clicking the Close button on the tab. However, the TabCloseMenu plugin uses the Remove method, which does not trigger the Close event, so I was forced to override all menu handlers and use the close() method instead of remove(). Does the use of remove have any special reason?

    I also expected the BeforeTabClose event to be triggered on TabPanel, but it doesn't bother me that much.

    5. CloseOthersTabsText should be CloseOtherTabsText.


    Versions: Ext JS 6.5.1.345 / Ext.NET 4.4.1


    Thank you for your assistance.

    Kind regards
    Dan
    Last edited by fabricio.murta; Aug 15, 2019 at 11:50 PM.
  2. #2
    Hi Dan,

    Thanks for the excellent sample demonstrating the issue. We will investigate right away.
    Geoffrey McGill
    Founder
  3. #3
    Hello Dan!

    Your reports are still actual as of Ext.NET 4.8.2.

    Quote Originally Posted by NewLink
    1. While CloseTabText works as expected, the CloseTabIcon setting does nothing.
    There's a bug here. I see the icon feature was "hand-merged" in the TabClose menu and we accidentally removed it while employing issue #1488. The good news is now we have a hang of what's been lost and will properly document, implement back, and use a better override system to allow the plugin to have our improvements -plus- be in sync with any changes or fixes it receives from Ext JS between updates.

    For this, we logged issue #1648 was created to track and address this issue.

    Quote Originally Posted by NewLink
    2. I tried to solve this in BeforeTabMenuShow.Handler, but the event does not fire.
    This is an event tied to the TabMenu extension for the TabPanel component, so it should not work in the context you shown.

    Quote Originally Posted by NewLink
    3. Then I tried to solve it by override:
    This looks like something that should work but, as the menu entries are not created with icons, they don't get an icon DOM element ready to change its icons at all, so the method does not work. You may either override the Ext.ux.TabCloseMenu.createMenu() method (and you'd have to override the whole method), or change your override to call:

    this.menu.child("#close").itemEl.addCls("icon-tabdelete");

    Quote Originally Posted by NewLink
    4. Finally, I'm quite confused about triggering events.
    Well, no need to quote further. You are confused and there's a good reason for this: close events in tab panels have issues for a long, long time, for the way the wrap ordinary panels around, the main report is logged under issue #115. You can get a little more information on the issue in a more recent issue, #1611: TabPanel DirectEvents not working.

    So this one is a little trickier to tackle, as it involves the very concept of the component, and been hanging around for so long (and Sencha didn't fix either).

    Quote Originally Posted by NewLink
    5. CloseOthersTabsText should be CloseOtherTabsText.
    This is how the setting is figured in Ext JS API and we simply replicate it. Although I agree this looks grammatically wrong.

    Hope this helps! We'll post a follow-up here as soon as we get issue #1648 addressed.
    Fabrício Murta
    Developer & Support Expert
  4. #4
    Hi FabrÃ*cio,
    thank you for a quick and comprehensive reply. Your answer is clear to me and I have only one small comment:

    5. CloseOthersTabsText should be CloseOtherTabsText.
    You are right that it is according to the API specifications (which I, unfortunately, did not read before), it just seemed strange that there is CloseOthersTabsText, but CloseOtherTabsIcon. It's just a little inconsistency.

    I greatly appreciate your help.

    Kind regards
    Dan
  5. #5
    Hello Dan!

    Quote Originally Posted by NewLink
    it just seemed strange that there is CloseOthersTabsText, but CloseOtherTabsIcon. It's just a little inconsistency.
    This setting is specific to Ext.NET and we just didn't want to spread around the bad grammar, possibly avoiding a breaking change in the future, as this from sencha is clearly a typo.

    I've noticed Sencha have been thoroughly reviewing their code for typos and also coding style lately. It raises a hope for them to use the switch from 6 to 7 to their advantage and introduce this breaking change now (instead of between minor releases!), so let's cross fingers, as the time to change that would be around now.

    On another topic, this reply is also to add as the promised follow-up that we've just included the fix in Ext.NET 5 Preview that we just released to NuGet. It reviews the implementation of the icons and also two more issues that were addressed by the version lost with issue #1488. To enumerate them:

    - Implement back the Icon and IconCls settings for Close, CloseAll and CloseOther tabs
    - Fixes an issue (needs confirmation) with menus hiding that kept the plugin's item selected, which incurred in inconsistencies elsewhere in the user experience
    - Replaces calls to remove() into closeTab() -- yes, this was already implemented before, and in accordance to your inquiry: but there should still have problems regarding events not triggering correctly from issue #115.

    Hope this helps!
    Fabrício Murta
    Developer & Support Expert
  6. #6
    We have just pushed Ext.NET 4.8.3 which includes the fix to the TabCloseMenu plugin, along with a fix to incompatibility with Internet Explorer (legacy) browsers.
    Fabrício Murta
    Developer & Support Expert
  7. #7
    Hello, just a small update related to one of the point you raised in this thread.

    I happened to report the issue with CloseOthersTabs up to Sencha, and they have this fixed. The perfect opportunity to introduce breaking changes is between major updates, so for Ext.NET 5, the setting will read after CloseOtherTabs instead.
    Fabrício Murta
    Developer & Support Expert
  8. #8
    Hello, just a small remark... the fix gh1648 introduced in Ext.NET 4.8.3 brought one mishap:

    onHideMenu: function() {
            var me = this;
    
            me.item = null;
            me.fireEvent('aftermenu', me.menu, me);
        }
    Setting me.item = null will cause the closing of the tab to not work correctly because the reference to the actual tab item is lost.

    Kind regards
    Dan
  9. #9
    Hello Dan!

    Thanks for the report, and yes, pretty basic thing of its functionality that went unnoticed with the fix. We'll take a look on that and arrange a proper fix.
    Fabrício Murta
    Developer & Support Expert
  10. #10
    Hello again!

    After creating issue #1681 to properly document the problem, we fixed it in both v4 and v5 branches. It seems just removing this old override is the way to go. The fix is immediately available in github and will be included in the next public releases of Ext.NET 4.x and 5.x series.

    Thanks again for reporting it.
    Fabrício Murta
    Developer & Support Expert

Similar Threads

  1. Replies: 3
    Last Post: Nov 13, 2015, 11:38 AM
  2. [CLOSED] TabCloseMenu Plugin.
    By CanopiusApplications in forum 2.x Legacy Premium Help
    Replies: 3
    Last Post: Dec 12, 2013, 6:19 AM
  3. Replies: 4
    Last Post: Nov 23, 2012, 6:36 PM
  4. [CLOSED] TabCloseMenu plugin firing close event
    By Justin_Wignall in forum 1.x Legacy Premium Help
    Replies: 2
    Last Post: Jul 28, 2010, 12:18 PM
  5. [CLOSED] [1.0] ValidationStatus plugin icons broken
    By tdracz in forum 1.x Legacy Premium Help
    Replies: 2
    Last Post: May 07, 2010, 8:25 PM

Posting Permissions