Only call drawTree() once when changing the layout
- Dominant language
- JavaScript
- Stars
- 56
- Forks
- 32
- PR merge metrics
- No merged PRs in 30d
Description
Brought up in https://github.com/biocore/empress/pull/363, although this has been a TODO in the code for a while now:
`Empress.reLayout()` calls `Empress.drawTree()`. After a call to `reLayout()`, in `Empress.updateLayout()`, `Empress.centerLayoutAvgPoint()` is called -- which calls `drawTree()` again. This represents a slight inefficiency.
There are cases where `reLayout()` is called by itself where we want to draw the tree afterwards without changing the camera position (e.g. when toggling ignore lengths vs. don't ignore lengths), and there are cases where `centerLayoutAvgPoint()` is called by itself (e.g. when clicking the "Reset camera" button).
Solutions to this include:
- remove `drawTree()` calls from both of these functions and just make the caller call `drawTree()` themselves if needed
- add boolean parameters to these functions that control whether or not to call `drawTree()`
- probs other things
Contributor guide
Research direction
Trace Empress.reLayout(), Empress.updateLayout(), and Empress.centerLayoutAvgPoint() to understand which callers need rendering and which only adjust the camera. Choose an approach that preserves standalone reLayout() and Reset camera behavior while ensuring a layout change calls drawTree() only once; verify the relevant interaction paths manually.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- frontend, performance
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100