@jimfb and @gaearon were kinda enough to reach out and warn us that RB will have a lot of warnings in React v15.2.0, of which, a release is imminent.

The problem is that in preparation for removing the DOM attribute whitelist, React has started to warn when props that are not DOM attributes are passed through to DOM nodes. This (AFAIK) to help for folks to clean up code before those attributes are suddenly applied to the DOM nodes in a later version when the whitelist is removed.

@react-bootstrap/collaborators If anyone has time to start talking this in part (or whole) this might be less of a chore if a bunch of folks work on it. We also need to decide what the best way to ensure that we do not break existing API's by being too restrictive in what we pass through.

Concerns

I'm not sure if any of these cases exist (here) but I am concerned about API's that require carte-blanche passthrough of props to a child that may or may not a DOM node. Presumably in those cases we can either defer to the User to use a component, or make sure to remove "own" props before passing through.

This also effectively means that API's like dropdown which allow for using the throwaway bsRole prop to indicate type to the parent no longer work for DOM nodes. That sort of sucks, that was a really great side-effect feature of that API.

Look like the recent react's update has cause react-bootstrap modal to stop working. Here are the errors:

This issue is also discussed on react-overlays

https://github.com/react-bootstrap/react-overlays/issues/188

Hey all,

Im trying to use the modal in a react class and I'm running into Cannot read property 'getDialogElement' of null and Cannot read property 'remove' of null.

here is my code snippet, lifted directly from your site, but adopted for a component:

import React from 'react';
import ReactDOM from 'react-dom';
import {Modal} from 'react-bootstrap';

export default class ModalDemo extends React.Component {

  constructor(props) {
    super(props);
    this.state = {
      showModal: false
    }
    this.close = this.close.bind(this);
    this.open = this.open.bind(this);
  }

  close() {
    this.setState({ showModal: false });
  }

  open() {
    this.setState({ showModal: true });
  }
  render() {
    return (
        <div>
          <p>Click to get the full Modal experience!</p>

          <Button
            bsStyle="primary"
            bsSize="large"
            onClick={this.open}
          >
            Launch demo modal
        </Button>

          <Modal show={this.state.showModal} onHide={this.close}>
            <Modal.Header closeButton>
              <Modal.Title>Modal heading</Modal.Title>
            </Modal.Header>
            <Modal.Body>
              <h4>Text in a modal</h4>
              <p>Duis mollis, est non commodo luctus, nisi erat porttitor ligula.</p>
            </Modal.Body>
            <Modal.Footer>
              <Button onClick={this.close}>Close</Button>
            </Modal.Footer>
          </Modal>
        </div>
    );
  }
}

When this code executes however I get the first error on show and the transition animation does not play and its this line var dialogNode = this._modal.getDialogElement(); in Modal.js that triggers it, because _modal is null. Breakpointing through the code, the issue seems to be happening because when the render method is called. When the page first loads, the render method is called, and this function ref={c => { this._modal = c; }} is invoked with c being the Modal object. However when the state changes (in this case pushing the button, c resolved to null and overrides the initial prop. Any functions that have to call Modal properties on _modal now fail.

It's very possible I missed something here and there is an issue with my code, but I can't seem to get this working properly within my context.

just compare with https://github.com/react-bootstrap/react-bootstrap/pull/1176 11 vs 22 exclusions :smile:

It seems semistandard is way less stricter and suits more for R-B.

I'm getting this error message

Error: Invariant Violation: addComponentAsRefTo(...): Only a ReactOwner can have refs. This usually means that you're trying to add a ref to a component that doesn't have an owner (that is, was not created inside of another component's render method). Try rendering this component inside of a new top-level component which will hold the ref.

when adding the following code:

<Navbar>
  <Nav navbar right>
    <NavItem onClick={this.props.handleCSV} href="#">Import CSV</NavItem>
  </Nav>
</Navbar>

Provide React 0.12 support in all react-bootstrap components.

BREAKING CHANGES: It's no longer possible to use key indiscriminately within React components, so I had to make the following changes to component props:

Panel: key -> eventKey MenuItem: key -> eventKey NavItem: key -> eventKey TabPane: key -> eventKey ListGroupItem: key -> eventKey

Comments or help finishing this off is welcome.

Describe the bug

When you click on <OverlayTrigger>button here</OverlayTrigger> it opens a popover, but an error appears in a console:

"Popper: CSS "margin" styles cannot be used to apply padding between the popper and its reference element or boundary. To replicate margin, use the offset modifier, as well as the padding option in the preventOverflow and flip modifiers."

To Reproduce

Steps to reproduce the behavior:

1. npx create-react-app sandbox
2. yarn add [email protected]
3. Use an OverlayTrigger and click on it.

Reproducible Example

Codesandbox, nor jsfiddle didn't work at the time i wrote this issue. Heres an example markup:

function App() {
  return (
    <div className="App">
      <OverlayTrigger
        trigger="click"
        placement="bottom"
        overlay={<h1>111111</h1>}
      >
        <button>PRESS ME!</button>
      </OverlayTrigger>
    </div>
  );
}

Click on the button and watch your console.

Expected behavior

No warning/error messages should be present in a console.

Screenshots

Environment (please complete the following information)

• MacOS 10.15.4
• Chrome 80
• React-Bootstrap Version 1.0.0

Per the documentation: http://react-bootstrap.github.io/components.html#panels, when using a panel, unless you specifically supply the header prop some heading tags, it will print the title area as normal text within the panel-heading div. If you wrap that Panel in an Accordion, supplying just a string to Panel's header prop will suddenly output a title wrapped in an H4.

I think this is inconsistent behavior. Is there any reason why it specifically creates an H4 tag in this instance? I would like to have an accordion panel where the heading text is just normal text and not an H4.

Panel.js:187 looks like it could be the culprit.

To reproduce create:

   Panel header='test title' 

Note that the title is rendered as normal text. Then try:

   Accordion
        Panel header='test title'
   /Accordion

The title now automatically becomes an H4

Moving the conversation from #811 into a PR b/c I think it may help provide some visibility into the code changes I am thinking about here. be warned this is a really long note

We have two major API's that are Changing: Modals, Overlays (tooltips/popovers)

Consolidations

• deprecations
  • OverlayMixin replaced by Portal component
• additions
  • Portal component, just renders its children into a container fixes #404

Modals and Overlays share some common functions underneath, and Users have come to use our OverlayMixin, the new Portal Component offers the same functionality in a way that works for both createClass or plain class consumers

  <Portal container={document.body}>
    <div>rendered into the body!</div>
  </Portal>

Modals

• deprecations
  • ModalTrigger - fixes #397, fixes #616
  • title and closeButton props on Modal (moved to ModalHeader)
  • onRequestHide prop renamed to onHide (more idiomatic with our callbacks)
• additions - partly addresses #748
  • ModalHeader
  • ModalTitle
  • ModalBody
  • ModalFooter

Api

new components are exported on the Modal component Automatically for ease of use, or can be imported from lib if the user wants. Body, Footer, and Title are just convenient wrappers of <div className='modal-part'/> and Modal Header will auto wire the closeButton click to the parent onHide.

<Modal container={mountNode} onHide={function(){}} aria-labelledby='title'>
    <Modal.Header closeButton>
      <Modal.Title id='title'>Modal title</Modal.Title>
    </Modal.Header>

    <Modal.Body>
        One fine body...
    </Modal.Body>
    <Modal.Footer>
        <Button bsStyle='primary'>Save changes</Button>
     </Modal.Footer>
</Modal>

I think its safe to remove (deprecated for now) ModalTrigger, it is a slight convenience over handling the state yourself ,but since you don't generally have 10s of modals in a component at once (as with tooltips potentially) I say we prefer the React philosophy of explicitness over abstraction here.

By explicitly specifying the Header, users get more customization options and better potential accessibility (aria-labelledby above). We can gracefully deprecate the current api, by checking if a user provides a title prop in the Modal and auto adding the new Header Component to the children, so no breaking changes.

Overlays

The tooltips and popovers (from here on out "Overlays") present some interesting issues Modals do not.

• You may have many Overlays on a page, and managing that manually in state is very verbose
• Overlays are targeted to a specific element, as well as potentially being rendered into a new subtree

Goals for the API

• maintain a similar level of ease-of-use that the OverlayTrigger offers, for wiring overlays to triggers and rendering custom overlays (non tooltips or Popovers)
• Allow better control over the provided Overlays so they can be tweaked for custom behavior, such as: #850, #816, #802, #611, #436

My Proposed path here is to ultmately end up with a declarative Overlay Component that can be completely controlled in terms of positioning, and display (show/hide). I think that looks something like the Modals above

<Button
  bsStyle='default'
  ref='btn'
  onClick={e => this.setState({ show: !this.state.show })}
>
  Holy guacamole!
</Button>
<Popover
  show={this.state.show}
  target={()=> this.refs.btn}
  placement='bottom'
  container={mountNode}
  containerPadding={20}
  title='Popover bottom'
>
  <strong>Holy guacamole!</strong> Check this info.
</Popover>

To acheive this though, both the render-to-container (show/hide), and positioning logic need to be removed from the OverlayTrigger Component and moved downstream. me and @taion have had a few long conversations on gitter about how far down to move it.

Before that though I have abstracted out that logic into seperate Components, we've already seen Portal as used by Modal, and the to-be-better-named: Position Component, for calculating an offset based on the target and container.

that leaves us with the following as the implementation of an Overlay:

class Popover extends React.Component {
  render(){
    let {
        container
      , containerPadding
      , target
      , placement
      , ...props } = this.props;

    return (
      <Portal show={props.show} container={container}>
        <Position {...{ container, containerPadding, target, placement }}>
          <PopoverMarkup>
            { this.props.children }
          </PopoverMarkup>
        </Position>
      </Portal>
    );
  }
}

Which feels like a lot of boilerplate for making custom Overlays. To help with that I am bundling both of these components up into an Overlay Component which makes it a bit better...

class Popover extends React.Component {
  render() {
    return (
      <Overlay {...this.props}>
        <PopoverMarkup {...this.props}>
          { this.props.children }
        </PopoverMarkup>
      </Overlay>
    );
  }
}

This feels a bit verbose to me but probably reasonable for Custom Overlays. It also means that the Popover and Tooltip Components are no longer "dumb" static markup components, which enables the declarative api way up page. I think that this is worth it, the Alternative is that we needto abandon the declartive api and put the logic back in OverlayTrigger or write this when we want the declarative syntax:

<Button bsStyle='default' ref='btn' onClick={e => this.setState({ show: !this.state.show })}>
  Holy guacamole!
</Button>
<Overlay
  show={this.state.show}
  target={()=> this.refs.btn}
  placement='bottom'
  container={mountNode}
  containerPadding={20}
>
  <Popover title='Popover bottom'>
    <strong>Holy guacamole!</strong> Check this info.
  </Popover>
</Overlay>

I find that less optimal than just having the PopOver component but it does have an advantage that @taion thinks is worth it. If we do it this way OverlayTrigger can jsut wrap its overlay prop in an Overlay Component, and then you can specify static markup to it, without it needing to be a special kind of Component.

Simplier Custom Overlays

  class MyOverlay {
    render(){
      let { positionLeft: left, positionTop: top } = this.props
      return <div style={{left, top}}>my overlay</div>
    }
  }

 <OverlayTrigger overlay={<MyOverlay/>}/>

which is less verbose than:

Simplier Declarative Overlays

  class MyOverlayMarkup {
    render(){
      let { positionLeft: left, positionTop: top } = this.props
      return <div style={{left, top}}>my overlay</div>
    }
  }

  // we could alternatively make this a function instead of a wrapper component like:
  // createOverlay(MyOverlayMarkup) or a decorator like @Overlay
  class MyOverlay {
    render(){
      return (
        <Overlay {...this.props}>
          <MyOverlayMarkup {...this.props}/>
        </Overlay>
      );
    }
  }

 <OverlayTrigger overlay={<MyOverlay/>}/>

GET TO THE POINT ALREADY THIS PR IS SOOOOOOOO LONG

Which Overlay Api do we go with?

• Simplier Custom Overlays
• Simplier declarative Popover/Tooltips
• Something completely different
• Just revert it all

Issue which keeps track of issues which are persistent / blocking before we can publish a version with Bootstrap 5 compatibility.

I can go ahead with creating the branch, bumping Bootstrap to v5 and connecting it with Netlify as soon as #5150 is merged. Then we have to go over the upstream changes etc.

Regarding Netlify and GitHub Pages, maybe use https://react-bootstrap-v5.netlify.app as soon its in alpha and once its stable move it over to the main name and v4 to https://react-bootstrap-v4.netlify.app.

cc @jquense @taion @bpas247

This PR attempts to resolve #1490. The main things that I changed are:

1. Added a parameter (eventType) to onToggle so that every time onToggle is fired you know exactly what event fired it ('click' if from handleClick, 'keydown' if from handleKeyDown, 'close' if from onClose, 'select' if from onSelect).
2. Added a parameter (source) to handleClose which is the source function that fired the handleClose.

I tried it out locally in my own app and I was able to override the default closing of the dropdown by creating my own state that I called dropdownOpen that I passed in as the open prop to the Dropdown component. I created a handleToggle function that I pass in as the onToggle prop which only closes when open is false and the event that closed it is close. Here's what my handleToggle looks like:

handleToggle(open, eventType) {
    // If open is false and type of event is 'close'
    if (!open && eventType === 'close') {
      this.setState({
        dropdownOpen: false,
      });
    }
    else {
      this.setState({
        dropdownOpen: true,
      });
    }
  } 

Here's what you get when you console.log the inputs to onToggle:

I didn't know how to write a test for this so if you could advise me on how I could then I'll add it to this PR, thanks!

Prerequisites

• [X] I am using the correct version of React-Bootstrap for my version of Bootstrap
• [X] I have searched for duplicate or closed issues
• [X] I have read the contributing guidelines

Describe the bug

The "sticky top" anchor in in https://react-bootstrap.github.io/components/navbar/#overview navigates to the path /#undefined which doesn't change anything (basically its an anchor tag linked to nothing):

https://react-bootstrap.github.io/components/navbar/#undefined

IMO the "Sticky top" anchor should be removed and let "Placement" without any sub-anchor, since it is already a section that fits alone one one's screen:

Expected behavior

Expected clicking on the link would navigate me to the specific section

To Reproduce

1. go to https://react-bootstrap.github.io/components/navbar/#overview
2. click on "Sticky top" in the nav
3. doesn't redirect

Reproducible Example

see above

Screenshots

see above

What operating system(s) are you seeing the problem on?

No response

What browser(s) are you seeing the problem on?

No response

What version of React-Bootstrap are you using?

2.7.0

What version of Bootstrap are you using?

5.2

Additional context

all above

Bumps json5 from 1.0.1 to 1.0.2.

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

Bumps json5 from 1.0.1 to 1.0.2.

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

Prerequisites

• [X] I am using the correct version of React-Bootstrap for my version of Bootstrap
• [X] I have searched for duplicate or closed issues
• [X] I have read the contributing guidelines

Describe the bug

Component with bootstrap modal being used is rendered slow and laggy on Chrome but works smoothly on Safari. Do you have any idea why or is this a bug?

Expected behavior

bootstrap modal works the same on all major browsers.

To Reproduce

No response

Reproducible Example

Will provide later.

Screenshots

No response

What operating system(s) are you seeing the problem on?

macOS

What browser(s) are you seeing the problem on?

Chrome

What version of React-Bootstrap are you using?

5.2.3

What version of Bootstrap are you using?

5.2.3

Additional context

No response

Is your feature request related to a problem? Please describe

I'm always frustrated when I can pass only some fixed variants to the button component.

Describe the solution you'd like

We should be able to pass rgb, rgba, hsl, hex or any color name to variant prop of the button component.

Describe alternatives you've considered

No response

Additional context

No response

It's better suited for this sort of thing, allows versioning and i18n, and supports the same sort of codeblocks, and react-metadata (b/c of plugins i wrote).

Downsides are that it's a bit harder to get a lot of these benefits if we want to build our own theme using react-bootstrap (It's generally my position that UI libraries should dogfood their own things).