`ReactLayersMixin`.
jordwalke opened this issue · 25 comments
I really like our ReactLayersMixin plugin and we should open source it. It allows us to implement not just the render function, but also the renderLayers function. The render is the same as its always been (describe your structure at any point in time. The renderLayers API allows you to describe your modal layers at any point in time. So that means you can open and close layers as a function of props/state just like your render() function can "react" to props/state.
Assigning to @yungsters who mastered the React/Layers stuff on FB's stack, but @petehunt also has great experience using and building out layers integration with React. So whoever has time/need.
FB layers has so many internal deps that we don't even use them at IG. Instead I suggest we open source the react layers stuff plus the light IG shim. You don't get the nice behaviors but the value for the community lies mostly in the architecture IMO.
@yungsters if we redid layers today would we do it any differently (ie I think we could do it with composition)? This could be a nice greenfield opportunity.
To be honest, I don't know if I would have done anything that differently. The Layer abstraction has held up really well — we have been able to extend it and make improvements over time without much code decay.
@yungsters yeah Layer is awesome -- what I'm wondering is why we use a mixin rather than composition to hook it up to React.
I guess I'm not sure what you mean by using composition to hook Layers up to React. Are you asking why we use renderLayers instead of returning the <Layer> components in render?
Yeah
Layers are not inherently anchored to any position in the DOM. For example:
render: function() {
return <div><button>Show Dialog</button><Dialog /></div>;
}
It does not make sense for <Dialog /> to exist in the returned tree. What we actually want is for render to return the subtree as well as a list of Layer components.
That makes sense, but the return value for renderLayers() is a little confusing (I've never used multiple layers in a component before), and it's not clear at first glance that it follows React's normal data flow.
When I designed renderLayers, I debated whether or not to allow returning a single Layer component. However, I did not want engineers to have to learn something new just because their use case requires more than one Layer component.
Also, I felt like it was not too painful to have to return a map with one key. Keep in mind that at the time, this was the standard way of passing in children with keys into a React component.
So I think that if we had frags I wouldn't mind putting the components in the render() method. While it would be kind of weird that it is not anchored to the DOM, if we had frags we would have precedent for components that don't render to anything in that place in the DOM and it would just be 1 paradigm to learn.
I would not be strongly opposed to that, but I don't think it makes the code easier to read. It makes the following questions harder to answer:
- Does this component render any Layers?
- Is any given component returned by
rendera Layer component? - Does this component render into any non-Layer?
I think it's odd to return things in the render() output that don't exist in the physical render tree. A component that has layers has the ability to have multiple distinct render tree roots. The ReactLayermixin in FB reflects that - but if the return value is too confusing, we can constrain it a bit - that should be very easy to do - we would just require that a single component be returned and modify existing code use. If we did allow them to be returned within frags, it would make sense that they are required to be at the top of the render tree - nothing deeper, and I'd probably be interested in making use of a more general abstraction that solves not only Layers but also other cases when you want a DOM subtree to actually exist on another part of the page, yet be controlled by your component. Think of it as a "Portal" to another part of the page:
But even just something as simple as our current ReactLayerMixin would be quite awesome. If released as a separate project, I wouldn't even mind bringing in a ton of extra JS resources. Pete, we could also just open source the Insta one quickly too, but I'd rather make it conform to ReactLayerMixin until we get frags, thoughts?
To be clear we do use ReactLayer and ReactLayeredComponentMixin at IG, just not any of the FB layer behaviors or Layer impl itself (yet), but they are API compatible for FB layers and behaviors by design. I am in favor of bringing all that to open-source, just want to make sure that we think about making any API cleanup before doing it.
Just spoke with @petehunt. Instead, we will publish a blog post with a rudimentary layer implementation and ReactLayersMixin that demonstrates how such a system would be built.
Not sure if this is the best place, but I'd love to see a blog post about this or some sample code!
@yungsters Any progress on this?
Closest thing available is http://khan.github.io/react-components/#layered-component
Thanks @petehunt
Can add that I found the discussion of react-bootstrap models and tooltips interesting: react-bootstrap/react-bootstrap#27
React Bootstrap has an implementation of this as well: https://github.com/react-bootstrap/react-bootstrap/blob/master/src/OverlayMixin.js
Hey! The company I work for is in currently in the process of a complete front-end overhaul using React.js and even some Flux Dispatchers and we're trying to move away from jQuery based overlay plugins and popups. We're currently using react-bootstrap's OverlayMixin to implement a tour, but I'm super curious to know what else the ReactLayersMixin might do. Even just a high level specification of what renderLayers does would be super helpful.
Specifically, we're having a lot of trouble figuring out when it's okay to start positioning/manipulating overlay nodes, esp. because you can't use the ref system for dom nodes not returned by render. We're also trying to figure out how best to deal with various problems with z-indexing and stacking contexts.
Thanks!
Hey, so i have created react-layers which is based off our learnings from working on react-bootstrap's OverlayMixin but i have reworked it to reflect the API's discussed here. It follows the react@0.11 API where you can return null from renderLayer which will result in the layer (and the node it's mounted in) being removed, this helps with the z-index issues as the last layer to show will always be the last child in the container.
Any feedback would be greatly appreciated! Especially around API learnings from FB or IG implementations.
@brainkim i have added some basic examples but over the next couple of days i plan on adding a example showing positioning/manipulating, maybe tooltips or a draggable popup.
@yungsters any update on the blog post? :)
I'm curious if renderLayer & renderSubtreeIntoContainer are still used to handle layering?
@mziemer21 the modern solution is something like react-portal.
Yea. We actually codemodded the mixin away just a month ago, and removed the distinction between render and renderLayers—layered components are just regular components now. It is a little awkward without fragments but no big deal.
I've spent some sensible time to research on this. As a result I end up with the library which permanently solved this problem for me. Check out, please, would be good to know the impressions and everything https://github.com/AlexeyFrolov/react-layer-stack