Material-ui: [Popover] Closed popover renders empty div

[fzaninotto]

Description

With the following sample code (taken from the doc:

      <div>
        <RaisedButton onTouchTap={this.handleTouchTap} label="Click me"  />
        <Popover
          open={this.state.open}
          anchorEl={this.state.anchorEl}
          anchorOrigin={{horizontal: 'left', vertical: 'bottom'}}
          targetOrigin={{horizontal: 'left', vertical: 'top'}}
          onRequestClose={this.handleRequestClose}
          animation={PopoverAnimationVertical}
        >
          <Menu>
            <MenuItem primaryText="Refresh" />
            <MenuItem primaryText="Help &amp; feedback" />
            <MenuItem primaryText="Settings" />
            <MenuItem primaryText="Sign out" />
          </Menu>
        </Popover>
      </div>

When the popover is closed, it renders as an empty div, which breaks the layout when I try to align more than one button with popover.

Affects <DropDownMenu>, <SelectField>, and <Popover>.

Images & references

Versions

• Material-UI: 0.16.4
• React: 0.15.4
• Browser: Chrome evergreen

[fzaninotto]

I think the regression was introduced by #5444, reverting #3806, which was a cure for the same problem (o_O).

[fzaninotto]

Any opinion on this one? I'd be happy to open a PR reverting the previous revert (!) to go back to the original version since that solves the issue.

As a side note, this blocks a new release of marmelab/admin-on-rest (offering React 15.4 compatibility).

[fzaninotto]

Sorry for asking again, but I'm concerned by this regression and the lack of response from the maintainers. I'm sure you guys are very busy, but this is a possibly blocking regression for many projects, with a one line fix.

[kkomaz]

+1 experiencing this as well......

[ericvrp]

We worked around this bug by changing the <div> at the beginning of this issue into <span className='LastChildDisplayNone'> and adding the following CSS.

.LastChildDisplayNone > div:last-child {
display: none;
}

[haraldronge]

Though you'd have to watch your Code once this bug is officially solved, otherwise your last child disappears from your UI.

[nathanmarks]

@fzaninotto @kkomaz Sorry for the slow response guys.

A lot of effort right now is being put into the next major version which has much better test and regression test coverage -- concerns in this area (regressions, consistency, etc) are major reasons for the upcoming major version.

That being said, this is definitely important for the current version. If you submit a PR, i'll 👍 it so it goes in the next patch release.

[ericvrp]

My workaround is a hack that should not go into a PR.

[oliviertassinari]

Sorry for asking again, but I'm concerned by this regression and the lack of response from the maintainers.

Sorry for the late answer, I don't receive any notification from issues. The signal vs noise is too high for what I can handle. I focus on PRs. Feel free to ping me directly next time 👍 .

Any opinion on this one? I'd be happy to open a PR reverting the previous revert (!) to go back to the original version since that solves the issue.

I can't remember the motivation behind that fix. Please, feel free to submit a PR adding that style back 👍 .