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

Batched the getBoundingClientRect and style changes and added an IntersectionObserver #957

Closed
wants to merge 2 commits into from

Conversation

cefaci
Copy link

@cefaci cefaci commented Feb 25, 2022

Hi! I experienced a lot of layout reflows causing by GetBoundingClientRect and style reads/writes while loading pages >10, so I tried to batch the GetBoundingClientRect calls and separate css reads/writes.
I added a second option with an IntersectionObserver.

@cefaci cefaci changed the title React pdf/main Batched the getBoundingClientRect and style changes and added an IntersectionObserver Feb 25, 2022
@wojtekmaj
Copy link
Owner

Hi! This is a neat contribution, but will be quickly overwritten by #944 🤔 On top of that, merging it would also require bumping major version, because IntersectionObserver still requires polyfill for some of browsers we support.

@wojtekmaj wojtekmaj added the enhancement New feature or request label Apr 5, 2022
@cefaci
Copy link
Author

cefaci commented Apr 6, 2022

Hi! Thanks, I will update it for #944 and could just push w/o IntersectionObserver and make the batch optional?

@wojtekmaj
Copy link
Owner

I believe it won't be necessary now that #944 is merged. Correct me if I'm wrong please.

@wojtekmaj wojtekmaj closed this Apr 20, 2022
@cefaci
Copy link
Author

cefaci commented Jul 12, 2022

I like it! Great work, thanks! Tested the beta.

I just passed me the element in Page/TextLayer.jsx too, to change the innerHMTL for e.g. highlighting later:

      cancellable.promise
      .then(() => {
        if (customTextRenderer) {
          Array.from(this.layerElement.current.children).forEach((element, elementIndex) => {
            const reactContent = customTextRenderer({
              element, // Here
              itemIndex: elementIndex,
              ...textContent.items[elementIndex],
            });
            element.innerHTML = renderToStaticMarkup(reactContent);
          });
        }
        this.onRenderSuccess();
      })
      .catch((error) => {
        this.onRenderError(error);
      });

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants