Inconsistent state after barplot errors
- Dominant language
- JavaScript
- Stars
- 56
- Forks
- 32
- PR merge metrics
- No merged PRs in 30d
Description
This is a small weird problem that nonetheless makes the UI look weird:

The problem is caused by setting the barplots to something invalid (e.g. trying to use continuous coloring with a categorical field, as shown in the GIF above), and then -- rather than fixing the error -- going and doing something else (e.g. changing the layout parameters).
What happens is the barplot state hasn't been reset to something that at least _works_, so -- although the canvas is changed -- I think what happens is stuff is bailed out of before `draw()` is called. This results in the weird scenario where the tree remains unchanged until the user does something like zooming or panning, as shown in the GIF above.
Some ways we could fix this:
- Detect when an error is thrown when trying to draw barplots (*and*, alternatively, when barplots are drawn successfully), and update an `Empress` (?) class-level boolean variable named `barplotsBroken` or something like that. Then, only [call `Empress.drawBarplots()` from `Empress.reLayout()`](https://github.com/biocore/empress/blob/df3a4da5a5b23d465937112dfa2050f8957bb0ed/empress/support_files/js/empress.js#L2714-L2718) if `barplotsBroken` isn't true. (Pressing the barplot panel `Update` button [calls `Empress.drawBarplots()` directly](https://github.com/biocore/empress/blob/df3a4da5a5b23d465937112dfa2050f8957bb0ed/empress/support_files/js/barplot-panel-handler.js#L132-L134), which gives us the chance to un-break the barplots.)
- Instead of throwing an error, immediately modify the barplot UI to automatically "fix" any erroneous settings and then use that -- e.g. uncheck the continuous values box, as is done in #484. (I think this would be difficult to implement smoothly for barplots, since there are more ways these can cause errors and messing with the UI automatically here might get confusing/annoying for users...)
I think the first option would be best, and shouldn't be too difficult to implement.
Contributor guide
Research direction
Reproduce the invalid barplot-setting flow in the UI, then read Empress.reLayout() in support_files/js/empress.js and the Update handler in support_files/js/barplot-panel-handler.js. Trace how drawBarplots() errors affect later layout redraws; done means changing layout after a barplot error updates the tree immediately, while the panel Update action can recover the barplots.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- data-visualization, frontend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 50/100