Giter Club home page Giter Club logo

Comments (4)

tajo avatar tajo commented on August 25, 2024

I don't like the prop rendered, that's not idiomatic (the wrapper is). Also, portal doesn't have its own wrapper. It renders null or openByClickOn element.

from react-portal.

slorber avatar slorber commented on August 25, 2024

@tajo about the prop name does not matter much to me :)

I can't use openByClickOn because I don't want the portal to open by clicking on the element.

Here is a complete sample from my codebase so that you can understand what iI'm trying to do. As only react portal needs a wrapper, I have to use the same wrapper everywhere to avoid inconsistencies. I'd rather avoid this if possible.

'use strict';

var _ = require('lodash');
var $ = require("jquery");
var React = require('react');
var ReactDOM = require("react-dom");

var TetherTarget = require('common/tether/tetherTarget');
var CenteredModal = require("components/portals/common/centeredModal");
var DOMUtils = require("lib/dom/DOMUtils");

var AppMediaqueries = require("appMediaqueries");

/*
This portal is a combination of a tether (positionned) portal and a normal portal
We try to open a positionned portal, and if we see that after mounting it renders outside of viewport,
we then fallback to a regular non-positionned centered portal
 */
var CleverTetherTarget = React.createClass({
    propTypes: {
        minimumViewportWidth: React.PropTypes.number.isRequired
    },
    getDefaultProps() {
        return {minimumViewportWidth: AppMediaqueries.SmallToLargeScreenBreakpoint}
    },

    getInitialState() {
        return {isFallback: false};
    },
    useFallback() {
        this.setState({isFallback: true});
    },

    componentDidMount() {
        // it's not a big deal here to query window width and don't handle resize events
        // Tether portals are generally short-lived so it's unlikely to fail (render outside of viewport)
        // and if it does user can simply re-open the portal in most cases
        if ( $(window).width() < this.props.minimumViewportWidth ) {
            this.useFallback();
        }
    },

    onTetherMount(tetherTarget) {
        if ( tetherTarget ) {
            const portalDomElement = tetherTarget.getPortal();
            const inViewport = DOMUtils.isElementInViewport(portalDomElement);
            if ( !inViewport ) {
                this.useFallback();
            }
        }
    },

    render() {
        if ( !this.props.tethered ) {
            return (
              <div className="clever-tether-target-wrapper">
                  {this.props.children}
              </div>
            );
        }
        else if ( this.state.isFallback ) {
            return (
              <div className="clever-tether-target-wrapper">
                  {this.props.children}
                  <CenteredModal>
                      {this.props.tethered}
                  </CenteredModal>
              </div>
            );
        }
        else {
            return (
              <div className="clever-tether-target-wrapper">
                  <TetherTarget {...this.props} ref={c => this.onTetherMount(c)}/>
              </div>
            );
        }
    }
});

module.exports = CleverTetherTarget;

Note that CenteredModal is a wrapper around react-portal and Tether is a library that I use to create portals that are positionned on top of children.

from react-portal.

tajo avatar tajo commented on August 25, 2024

I see but I still don't get why this

Currently to do so I have to wrap it inside a div, because React can't render an array

bothers you so much. It's one of the React axioms. I'm pretty sure that every React app has many components with unnecessary div wrappers. That's how it is. Passing React elements through a prop is no go. openByClickOn is the only exception because it's so useful (but still not something that should be very common). My opinion...

from react-portal.

slorber avatar slorber commented on August 25, 2024

@tajo making React able to render an array has been on the React team roadmap for a while already (as far as I remember): facebook/react#2127

There are many cases where doing so can be useful. For example: http://stackoverflow.com/questions/30976722/react-performance-rendering-big-list-with-purerendermixin

Having a wrapper div is not such a big deal for me, but still if we can avoid creating useless wrappers it's preferable. I had to change a bit some CSS because of this wrappern and it has a minor performance/memory cost

Passing React elements through a prop is no go

Also don't understand why it bothers you much to provide this ability :) It does not cost much to allow a prop to be used instead of "null". Basically it just means replacing return null; by return this.props.elementToRender || false

And yes, it is likely that very few people will use this, because the only case where you don't have a wrapper to your portal is basically when you are building a portal abstraction on top of react-portal. I'd like to publish someday that abstraction and have react-portal as a dependency but don't like to force the user to have this wrapper element.

from react-portal.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.