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

Popup rerenders and slowing page #3338

Closed
DavidLozzi opened this issue Dec 13, 2018 · 5 comments
Closed

Popup rerenders and slowing page #3338

DavidLozzi opened this issue Dec 13, 2018 · 5 comments

Comments

@DavidLozzi
Copy link

DavidLozzi commented Dec 13, 2018

Bug Report

Steps

I have Popup on our page, about ~80 times (20 database results, with 4 popups each). We have SEVERAL other Semantic UI components on the page as well, being repeated just as much.

When I interact with another control on the page, the page is sluggish, and exploring performance it shows the Popup control rerendering, always.

Here is the full render event when I click in a text box
image

Zooming into the depths, I see only Popups, no other controls (except for the parents of course)
image

We implemented Popup via our own component, using memo

import { memo } from 'react';
import { Popup } from 'semantic-ui-react';
import './Popup.scss';

export default memo(Popup);

And one example of the implentation

import Popup from 'components/Popup/Popup';
...
Card.Download = class CardDownload extends PureComponent {
  downloadFileHandler = () => {
...
  };

  render = () => {
    const {
      display
    } = this.props;

    return (
      display &&
      <div className="imgWrapper">
        <Popup
          trigger={<Image className="actionIcon" onClick={this.downloadFileHandler} src={downloadIcon} />}
          content="Download"
          position="bottom right"
          hideOnScroll
        />
      </div>
    );
  }
};

Expected Result

Not to have Popup rerender completely.

Actual Result

See images above

Version

0.82.5

Testcase

@welcome
Copy link

welcome bot commented Dec 13, 2018

👋 Thanks for opening your first issue here! If you're reporting a 🐞 bug, please make sure you've completed all the fields in the issue template so we can best help.

We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.

@stale
Copy link

stale bot commented Jun 11, 2019

There has been no activity in this thread for 180 days. While we care about every issue and we’d love to see this fixed, the core team’s time is limited so we have to focus our attention on the issues that are most pressing. Therefore, we will likely not be able to get to this one.

However, PRs for this issue will of course be accepted and welcome!

If there is no more activity in the next 180 days, this issue will be closed automatically for housekeeping. To prevent this, simply leave a reply here. Thanks!

@stale stale bot added the stale label Jun 11, 2019
@qyangdu
Copy link

qyangdu commented Jun 27, 2019

have similar problem, Popup use a lot CPU time comparing to other controls.

@stale stale bot removed the stale label Jun 27, 2019
@layershifter
Copy link
Member

@DavidLozzi @qyangdu in 0.87.0 we switched to Popper.JS (see #3532), now Popup is more performant. Any style computations will be applied only to opened Popup, please check and provide the feedback.

FYI: I tried it on Icon page, https://react.semantic-ui.com/elements/icon/ No rerenders 🗡

@qyangdu
Copy link

qyangdu commented Aug 5, 2019

Yes the performance is much better, but it brings in the issue #3725.

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

3 participants