Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ReactLayersMixin. #379

Closed
jordwalke opened this issue Sep 27, 2013 · 25 comments
Closed

ReactLayersMixin. #379

jordwalke opened this issue Sep 27, 2013 · 25 comments
Assignees

Comments

@jordwalke
Copy link
Contributor

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.

@ghost ghost assigned yungsters Sep 27, 2013
@petehunt
Copy link
Contributor

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.

@yungsters
Copy link
Contributor

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.

@petehunt
Copy link
Contributor

@yungsters yeah Layer is awesome -- what I'm wondering is why we use a mixin rather than composition to hook it up to React.

@yungsters
Copy link
Contributor

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?

@petehunt
Copy link
Contributor

Yeah

@yungsters
Copy link
Contributor

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.

@petehunt
Copy link
Contributor

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.

@yungsters
Copy link
Contributor

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.

@petehunt
Copy link
Contributor

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.

@yungsters
Copy link
Contributor

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?

@jordwalke
Copy link
Contributor Author

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?

@petehunt
Copy link
Contributor

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.

@yungsters
Copy link
Contributor

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.

@bywo
Copy link

bywo commented Feb 6, 2014

Not sure if this is the best place, but I'd love to see a blog post about this or some sample code!

@abergs
Copy link

abergs commented Aug 12, 2014

@yungsters Any progress on this?

@petehunt
Copy link
Contributor

Closest thing available is http://khan.github.io/react-components/#layered-component

@abergs
Copy link

abergs commented Aug 12, 2014

Thanks @petehunt

Can add that I found the discussion of react-bootstrap models and tooltips interesting: react-bootstrap/react-bootstrap#27

@pieterv
Copy link
Member

pieterv commented Aug 12, 2014

React Bootstrap has an implementation of this as well: https://github.com/react-bootstrap/react-bootstrap/blob/master/src/OverlayMixin.js

@brainkim
Copy link
Contributor

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!

@pieterv
Copy link
Member

pieterv commented Aug 26, 2014

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 [email protected] 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.

@brainkim
Copy link
Contributor

@pieterv Great! Will take a look at it and get back to you!

@mziemer21
Copy link

@yungsters any update on the blog post? :)
I'm curious if renderLayer & renderSubtreeIntoContainer are still used to handle layering?

@brigand
Copy link
Contributor

brigand commented Aug 3, 2016

@mziemer21 the modern solution is something like react-portal.

@gaearon
Copy link
Collaborator

gaearon commented Aug 3, 2016

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.

@lexfrl
Copy link
Contributor

lexfrl commented Oct 2, 2016

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests