Giter VIP home page Giter VIP logo

Comments (24)

stevoland avatar stevoland commented on April 27, 2024

My idea for the project is to re-implement the Bootstrap component functionality as far as feasible using pure React. I certainly don't want to use the Bootstrap JS or any JS that requires jquery. We will probably have to do positioning etc with DOM manipulation. Tether looks good but I would prefer to JS dependencies at all - might it be worth ripping bits out of it?

Popovers and Modals are indeed difficulty to do in React. I'm unsure how to begin approaching them. Hoping the much whispered about Facebook 'Layers' thingy will help...

from react-bootstrap.

syllog1sm avatar syllog1sm commented on April 27, 2024

Aha! I'm on-board with that ambition. Awesome.

I favour shipping the functionality via dependencies first, and replacing them with React as we go.

Personally I don't see zero dependencies as a must-have, only a nice-to-have. I like not depending on jQuery simply as a statement of intent about how the library is to work. It says quite clearly, "the components do not communicate over the DOM".

But, for other utilities, I'm less sold. I think if Tether or something else is small and really solves a problem for us, I'd prefer to just depend on it, rather than partially embed it.

from react-bootstrap.

syllog1sm avatar syllog1sm commented on April 27, 2024

As for the implementation of modals, I've been thinking about it, and I see two strategies:

  1. Allow the modal pane to be a DOM-sibling of the trigger, and modify all relevant CSS properties of their parent when the modal is toggled. For instance, if position: absolute is a problem, it's toggled off when the modal is activated. I'm not a very experienced front-end dev, but it looks to me like this is quite filthy, and will be a nightmare across browsers.

  2. Attach the modal pane to the document body, and wire it up to the trigger via refs and callbacks. The communication chain in React will be fiddly, but this will be an implementation-internal issue. I think this will involve some black magic, but will be a more solid implementation.

I wonder whether, if we ask nicely, we can get a pre-release code-dump of the Layers thing. I'd be happy to read the source and figure it out without docs.

from react-bootstrap.

stevoland avatar stevoland commented on April 27, 2024

Aha! I'm on-board with that ambition. Awesome.

Cool!

But, for other utilities, I'm less sold. I think if Tether or something else is small and really solves a problem for us, I'd prefer to just depend on it, rather than partially embed it.

Yeah, I can be persuaded, especially where it involves browser quirks etc. I really don't want to have to deal with that stuff

Last I heard about the Layers doo-dah was that they'd just be writing about the approach, not releasing code but pieterv tells me that won't be soon.

from react-bootstrap.

pieterv avatar pieterv commented on April 27, 2024

I have been having a play with some possible API's for tooltips and popovers and think i have found one that will be flexible enough to allow the use of Tether or any external positioning lib but simple enough to just work easily.

I think the first step should be to abstract the display of the overlays from the positioning of them. I imagine they could have a very simple API as below:

var pos = {left: 10, top: 10};

var tooltipInstance = (
      <Tooltip placement="right" position={pos}>
        <strong>Holy guacamole!</strong> Check this info.
      </Tooltip>
  );

React.renderComponent(tooltipInstance, mountNode);

If we have a simple standard API for all overlays, we can then make another component which is just concerned with positioning elements relative to another element.

var tooltipInstance = (
    <Tooltip>
        <strong>Holy guacamole!</strong> Check this info.
    </Tooltip>
  );

var positionerInstance = (
    <OverlayPositioner placement="right" overlay={tooltipInstance}>
      <Button>Holy guacamole!</Button>
    </OverlayPositioner>
  );

React.renderComponent(positionerInstance, mountNode);

With this design it we could allow 3rd parties to create say a <TetherOverlayPositioner /> that provided the more advanced Tether functionality, but we provide a "good enough" for most cases positioner within core based on bootstrap.

I have had a look over the bootstrap code and i see no obvious reason why we couldn't replicate its functionality without using jquery.

How does this API sound? I will create a example implementation and submit a pull request so you guys can see how it feels :)

from react-bootstrap.

syllog1sm avatar syllog1sm commented on April 27, 2024

Hmm.

I'd really rather not make clients think about stuff like, "what positioning algorithm do I want?"

I'd rather tell them, "this is the positioning algorithm". If anyone wants to do something different, it's not very much code to make that happen without us shipping any particular functionality for it.

I guess what I'm saying is, I think it's best if uncertainty about this doesn't touch the API in any way. If we're worried about supporting an extra dependency, I'd rather just have two versions implemented, and degrade to the zero-dependencies algorithm if the client doesn't want to pull in an extra dependency.

Purely on API merit, I'd rather have the client writing something like:

<Button overlay={content: function() {...}, placement: <top | right | left | bottom | modal>, trigger: <hover | click>}>
  ...
</Button>

Another concern is that the OverlayPositioner component provides logic, not an on-screen element. I doubt we'd have more than one or two other components that purely package up logic like that. If it'll be exceptional, we should avoid it.

from react-bootstrap.

pieterv avatar pieterv commented on April 27, 2024

I'd rather tell them, "this is the positioning algorithm". If anyone wants to do something different, it's not very much code to make that happen without us shipping any particular functionality for it.

Totally agree with this, i would only suggest adding the one positioner to core, any additional positioning components would need to be created in the user space. Also thinking maybe Positioner isn't quite the best name for this component but not sure what else would give an idea of what its doing but not make someone think about "what positioning algorithm do I want?" unless they want to.

The reason i didn't opt for the following API was the limitations it placed on the components it could attach to, for example if you wanted to display anything other than a button, how would you do this? The only way i can think of would be to pass the component as a property, but then we might as well use props.children for this.

<Button overlay={content: function() {...}, placement: <top | right | left | bottom | modal>, trigger: <hover | click>}>
  ...
</Button>

With the API i have proposed there would be nothing stoping us from packing up components for commonly needed configurations while still exposing the underlying components they are made up of. A commonly needed component maybe:

var LinkWithTooltip = React.createClass({
  render: function () {
    return (
        <OverlayPositioner placement="top" overlay={<Tooltip>{this.props.tooltip}</Tooltip>}>
          {this.transferPropsTo(<a>{this.props.children}</a>)}
        </OverlayPositioner>
      );
  }
});

// Example usage
<p>
  Tight pants next level keffiyeh
    <LinkWithTooltip tooltip="Some awesome link" href="#">you probably</LinkWithTooltip>
  haven't heard of them.
</p>

I have got pretty far with this API implementation but am a little stuck on hard part of where to put the DOM elements for the popups, i am currently looking at a nice way to inject them onto the body as the bootstrap plugin does.

from react-bootstrap.

syllog1sm avatar syllog1sm commented on April 27, 2024

The reason i didn't opt for the following API was the limitations it placed on the components it could attach to, for example if you wanted to display anything other than a button, how would you do this? The only way i can think of would be to pass the component as a property, but then we might as well use props.children for this.

I was hoping that we could have BootstrapMixin, or some other mixin, manage the overlays.

Perhaps we could read the overlay property, and if it's set we could wrap the component in the OverlayPositioner on rendering. The OverlayPositioner component is there to be used directly if the client wants, but the default is to write things like .

I have got pretty far with this API implementation but am a little stuck on hard part of where to put the DOM elements for the popups, i am currently looking at a nice way to inject them onto the body as the bootstrap plugin does.

I had a lot of trouble with this when I was fiddling with modals. Eventually I ended up with the content as a child of the trigger, but this led to further bugs down the line.

A subtle complication with these things is that you often want to be able to click anywhere to dismiss an overlay. This means that the top-level component needs to know which overlays are open. The only other way I can see is to attach event handlers directly to the document body, which React discourages.

If we're going to ask the client to add logic to the top-level component, we'll probably want to supply a mixin, and tell them to add it to their top-level component. We could then have them name their top-level render method as something else, like renderPage, which our mixin would call. This would let us inject a region to store hidden elements. We'd get something like this:

var PageMixin = {
  render: function() {
    return (
      <div>
        <HiddenElements ref="hidden"/>
        {this.renderPage()}
      </div>
    );
  }
};

I'm not quite sure how to allow arbitrary elements down the tree to write to "hidden" without passing extra properties/callbacks around though.

from react-bootstrap.

syllog1sm avatar syllog1sm commented on April 27, 2024

Oh, forgot. One other suggestion.

It probably is possible to get the overlay to be a sibling/child of the trigger. If it comes down to it, we could just do that, and hack around the various issues that arise...

from react-bootstrap.

pieterv avatar pieterv commented on April 27, 2024

It probably is possible to get the overlay to be a sibling/child of the trigger. If it comes down to it, we could just do that, and hack around the various issues that arise...

Thats what i have implemented currently which works great until you put the overlay inside a inline element like a <p> since the Tooltips use <div>'s. I can't see any other work around for that other than walking up the tree and inserting the overlay at the first block level element, but im not sure if react will like that very much :P

If we're going to ask the client to add logic to the top-level component, we'll probably want to supply a mixin, and tell them to add it to their top-level component.

Hmmm yeah that sounds right. As @stevoland said i would really love to see the result of this issue facebook/react#379, as it sounds like they have already elegantly solved this problem.

from react-bootstrap.

stevoland avatar stevoland commented on April 27, 2024

Hot off the petehunt press: http://jsfiddle.net/LBAr8/

from react-bootstrap.

ssorallen avatar ssorallen commented on April 27, 2024

@stevoland Hunt's Mixin looks great. I created an anchor in the DOM manually for a Bootstrap Modal I created, but creating containers on demand makes more sense. This uses jQuery but doesn't really need to; it's working right now: https://github.com/mesosphere/marathon/blob/master/src/main/resources/assets/js/components/ModalComponent.js

from react-bootstrap.

stevoland avatar stevoland commented on April 27, 2024

I've started on a Modal based on petehunt's fiddle: https://github.com/stevoland/react-bootstrap/tree/feature/modal. I'm thinking that instead of a LayeredComponentMixin we just expose a ModalMixin that handles the component just implements renderModal() and state.modalShown.

Any thoughts?

from react-bootstrap.

pieterv avatar pieterv commented on April 27, 2024

If you keep the LayeredComponentMixin general like it is then i can use it for my popover/tooltip implementation as well? I guess it makes sense to also expose a nice Modal based API for creating modals as well.

I do wonder if we could come up with a nice way of creating Modals without a user having to create a new react component for basic modals. It would be nice to make it as simple as possible to use. Could maybe an API like this work?

var modalInstance = (
    <Modal>
        <strong>Holy guacamole!</strong> this is modal content.
        <Button bsStyle="default">click to do thing</Button>
    </Modal>
  );

var modalLinkInstance = (
    <ModalLink modal={modalInstance}>
      <Button>Holy guacamole!</Button>
    </ModalLink>
  );

React.renderComponent(modalLinkInstance, mountNode);

from react-bootstrap.

stevoland avatar stevoland commented on April 27, 2024

If you keep the LayeredComponentMixin general like it is then i can use it for my popover/tooltip implementation as well?

Yes, definitely

I guess it makes sense to also expose a nice Modal based API for creating modals as well.

Yeah,this needs thinking about

Could maybe an API like this work?

That's nice, yeah - I'll do that.

from react-bootstrap.

stevoland avatar stevoland commented on April 27, 2024

@pieterv with your ModalLink API, how does the modal request to be closed? I thought it would be possible but now my head hurts.

from react-bootstrap.

stevoland avatar stevoland commented on April 27, 2024

ok, I've got it now. ModalLink clonesWithProps this.props.modal adding onRequestClose and shown props.

The modal content can close itself like:

var closeAfterDoingSth = function () {
    this.refs.modalLink.close();
};

var modalInstance = (
    <Modal>
        <strong>Holy guacamole!</strong> this is modal content.
        <Button bsStyle="default" onClick={closeAfterDoingSth}>close after doing something</Button>
    </Modal>
  );

var modalLinkInstance = (
    <ModalLink modal={modalInstance} ref="modalLink">
      <Button>Holy guacamole!</Button>
    </ModalLink>
  );

React.renderComponent(modalLinkInstance, mountNode);

from react-bootstrap.

stevoland avatar stevoland commented on April 27, 2024

Think I'll implement OverlayTrigger that can be used for modals, popovers and tooltips. Will also change renderLayer to renderOverlay and LayeredComponentMixin becomes OverlayTriggerMixin.

<OverlayTrigger overlay={modalInstance} trigger="hover">
    <Button>Holy guacamole!</Button>
</OverlayTrigger>

trigger defaults to click. Sweet. Just need to figure out how to fade out a component when its unmounted.

from react-bootstrap.

pieterv avatar pieterv commented on April 27, 2024

Modal and OverlayTrigger components look good. I will create a pull request with the Tooltip and Popover components soon :)

from react-bootstrap.

stevoland avatar stevoland commented on April 27, 2024

Brilliant. It's really coming together. Just got to find time to get tests done :)

from react-bootstrap.

pieterv avatar pieterv commented on April 27, 2024

Writing the tests will be a good chance to do a bit of a review of the older components too :)

I have also nearly got the router/static html generation stuff working for the docs site, so should be able to pull that in pretty soon. Just need someone to write the docs after that :P

from react-bootstrap.

syllog1sm avatar syllog1sm commented on April 27, 2024

Awesome!

I've actually been waiting for the docs generation stuff to come up. I'm
keen to do docs writing.

On Wed, Mar 5, 2014 at 11:43 AM, Pieter Vanderwerff <
[email protected]> wrote:

Writing the tests will be a good chance to do a bit of a review of the
older components too :)

I have also nearly got the router/static html generation stuff working for
the docs site, so should be able to pull that in pretty soon. Just need
someone to write the docs after that :P

Reply to this email directly or view it on GitHubhttps://github.com//issues/27#issuecomment-36697845
.

from react-bootstrap.

pieterv avatar pieterv commented on April 27, 2024

@syllog1sm Awesome :)

Will let you know when its ready!

from react-bootstrap.

stevoland avatar stevoland commented on April 27, 2024

Closing. We've implemented overlays without Tether

from react-bootstrap.

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.