Give PersistedElement a key #2036
Conversation
So there can be more than one on a page
|
|
||
| return <div ref={this.collectChildContainer}></div>; | ||
| } | ||
| } | ||
|
|
||
| PersistedElement.propTypes = { |
There was a problem hiding this comment.
why not the inline
static propTypes = {
...
};
syntax which it seems most of our ES6 classes use?
There was a problem hiding this comment.
(more readable to have it at the top of a class rather than not in it and completely below it IMO)
There was a problem hiding this comment.
hmm, I thought we were mostly doing it this way. This probably is more legible though.
There was a problem hiding this comment.
static propTypes: 26 instances
.propTypes = {: 40 instances
so yes you're right, but I believe most of them are from before people realised babel lets us use define inline static variables and is more readable. Feel free to ignore though as you're right with mostly the old way
|
|
||
| return <div ref={this.collectChildContainer}></div>; | ||
| } | ||
| } | ||
|
|
||
| PersistedElement.propTypes = { |
There was a problem hiding this comment.
(more readable to have it at the top of a class rather than not in it and completely below it IMO)
|
Wouldn't this only be a problem if there are two in the same place though? In which case the browser will revert to document tree order which is how they'd behave normally? |
|
Actually I guess the document tree order wouldn't match since it would depend on the order they were instantiated. Is the z-index here actually necessary for persistedelements or is it a stickerpicker thing? |
Persisted elements by nature exist in their own DOM tree, so if they're positioned over a DOM tree that contains elements with the z-index specified, i.e. the context menu in the sticker-picker's case, it will need to have it's z-index set accordingly. This is going to get messy quite quickly if the z-index isn't added to the css of the elements that need it. Managing it in the CSS is hard enough as it is. |
|
I'm not sure I follow. Isn't the desired effect to have the regular document, then any PersistedElements just above that, then any other things which float above, eg. ContextMenus? In which case all PersistedElements will have the same z-index. (All of this assumes they share a stacking context of course). |
|
IRL summary: the thing I was missing was that StickerPicker appears within a context menu which itself floats above the document, so it needs a z-index higher than that of the context menu. |
As it's not a thing that's necessary for other PersistedElements, only the sticker picker because it has to sit above the ContextMenu it sits in.
|
@lukebarnard1 ptal |
So there can be more than one on a page