saif-ellafi / foundryvtt-minimal-ui

Foundry VTT Module. Configurable UI module, allows the user to hide, collapse or auto-hide components separately.
MIT License
13 stars 10 forks source link

Question about section of code in minimalui.js #71

Closed jrdstauffer closed 2 years ago

jrdstauffer commented 2 years ago

May I ask what the code on lines 909-917 (version 1.4.0) / lines 1148-1156 (version 1.1.8) of minimalui.js is for? This code is activating the tabs on the sidebar which opens the section as a popout. My module, Sidebar Expander, tries to prevent this behavior but Minimal UI is forcing it to still happen.

        Hooks.once('ready', async function () {
            $("#sidebar-tabs > a:nth-child(n)").click(function (eve) {
                if (eve.currentTarget.classList.contains('collapse')) return;
                const tabName = jQuery(eve.currentTarget).attr('data-tab');
                if (ui.sidebar._collapsed) {
                    ui.sidebar.activateTab(tabName);
                }
            });
        });
saif-ellafi commented 2 years ago

Hey there @jrdstauffer - sorry for the delay. That seems to be a very old chunk of code, intended to reopen sidebar applications that are already "active" when the sidebar is collapsed.

Image you click on the Actor sidebar, and it opens the actor app (floating if sideebar is collapsed), then, if you close it, the actor tab will still remain active, so in Core Foundry, if you click it again, nothing will happen. This hacks it so it reactivates a tab either way. There is probably a nicer way to do this, let me know how this conflicts we can figure out something in common.

saif-ellafi commented 2 years ago

It could also be the case that Foundry V9 does no longer suffer from this issue., as I have reported it in the past. Edit: Indeed I believe in V9 this is not needed anymore.

jrdstauffer commented 2 years ago

Thanks for the explanation. Makes sense to me what you did it for. Since you discovered it's no longer needed in v9, do you plan on removing it? That would take care of the issue going forward and for Foundry version 0.8.9 since Minimal UI is locked at version 1.1.8 I can advise users to comment out that segment of code since I can't think of a better way for you to do it and eventually everyone will move to v9 so I don't think it's worth the effort to try to worry about updating the prior version. This is of course assuming you plan on removing it since it's no longer needed.

saif-ellafi commented 2 years ago

Will do, and release soon! Indeed only for v9

jrdstauffer commented 2 years ago

Thank you very much!