Adds a "pass thru" virtual element#29
Conversation
|
Here's the original motivation from phosphorjs/phosphor#436 (comment):
|
|
And here's a summary of the contents of the PR from phosphorjs/phosphor#437 (comment):
|
issues with vdom siblings still remain. siblings will get clobbered if/when the vdom updates. Still works well enough when the hpass element is the first sibling
the value of `iconPass` is used to initialize an hpass vdom element in eg the tabbar renderer
…ndling accomplished this in part by moving a bunch of the complexity to the creatDOMNode function
…ring still need to figure out how to handle cleanup; current implementation likely causes a memory leak due to uncleaned-up React components
still getting some unittest failures in FocusTracker and TabBar in Widgets
71f427e to
9922f3a
Compare
blink1073
left a comment
There was a problem hiding this comment.
Thanks for updating the signature to match h(), nice symmetry.
|
Released |
Awesome. That's going to save me a lot of fiddling with |
| if (title.iconRenderer) { | ||
| return hpass('div', title.iconRenderer); | ||
| } else { | ||
| return h.div({className}, data.title.iconLabel); |
There was a problem hiding this comment.
If the className is completely disregarded when title.iconRenderer is set, then maybe this.createIconClass(data) should not be called at all?
There was a problem hiding this comment.
@vidartf You're right, this is inconsistent, and kind of a holdover from an earlier point in the PR. I thought about it for a bit, and what I think should be done is that the className and label should be passed to the hpass element as well, to be set on its parent element. There's a PR for it at #36
Fixes phosphorjs/phosphor#395, phosphorjs/phosphor#436, and probably a bunch of other issues.
This PR adds the hpass function and VirtualElementPass class described in phosphorjs/phosphor#436.
This PR was originally phosphorjs/phoshor#437. @blink1073 Since you reviewed the original PR, could you please take a look at this one too?