biocore / empress

A fast and scalable phylogenetic tree viewer for microbiome data analysis
BSD 3-Clause "New" or "Revised" License
48 stars 31 forks source link

Need to un-hide "Update" buttons when enabling sample / feature metadata coloring tabs after finishing an animation #532

Open fedarko opened 3 years ago

fedarko commented 3 years ago

Small bug introduced by the changes in #505. As a part of this PR, the sample and feature metadata tree coloring tabs are disabled during EMPress / EMPeror animations and re-enabled when these animations are stopped. (This is a good thing!) If sample / feature metadata coloring was already active before the animations, it will not be automatically added when the animations are stopped (which is completely fine).

However, the Update buttons remain hidden for these tabs after they are re-enabled. Since these buttons are missing, in order to use sample / feature metadata coloring again (depending on which one was active right before the animation started), the user needs to un-check and then re-check the corresponding checkbox in order to get the Update button for that tab to show up again. (Alternatively, the user can enable the other tree coloring option's checkbox, which'll achieve the same effect.)

The GIF below shows this in the case of feature metadata coloring below (but, as mentioned above, the bug also exists for sample metadata coloring)—

animbug

Things worth noting

  1. This problem doesn't impact the shearing tab (at least as far as I can tell). This is because this tab does not have an update button (...so now I feel silly for lobbying for one here earlier :)

  2. This problem has already been addressed in the context of the EMPeror callbacks that @ElDeveloper set up a while back: https://github.com/biocore/empress/blob/ebbc1e7947f997a17d927c8f3b89e938e3e4e9bb/empress/support_files/js/emperor-callbacks.js#L70-L72

    ... So I think the way to fix this bug will be doing something similar (I think we'd need to give the EnableDisableSidePanelTab objects access to SidePanel since that owns the update buttons? there's probably a better way to do this, though.)

  3. If we want to be a bit lazy, we could just remove all of the logic that hides Update buttons -- this way there's no need to keep track of all this stuff, and this bug is automatically fixed. (This is the approach the barplot UI takes.)