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

Lazy draw dropdowns to improve performance #2925

Merged
merged 3 commits into from
Oct 13, 2021

Conversation

dsevillamartin
Copy link
Member

Fixes #876

Changes proposed in this pull request:

  • Add lazyDraw attribute to Dropdown component
  • Make PermissionGrid dropdowns use lazy draw

Reviewers should focus on:

  • Do we like this implementation? Did we have something else in mind?
  • Could this potentially be breaking if an extension has logic that occurs on the items() call of a Dropdown?

Confirmed

  • Frontend changes: tested on a local Flarum installation.

@dsevillamartin dsevillamartin marked this pull request as ready for review June 20, 2021 18:52
@SychO9 SychO9 requested review from SychO9 and davwheat August 11, 2021 13:33
js/src/common/components/Dropdown.js Outdated Show resolved Hide resolved
@davwheat davwheat added this to the 1.1 milestone Aug 11, 2021
@davwheat
Copy link
Member

I assume the comment about breaking exts was meant for getMenu?

I think we should take that risk. getMenu was called whenever view was called, and I can't see it being done anywhere else. If for some crazy reason someone extended getMenu with anything not related to just getting the menu, they should be able to move that logic to view instead.

@SychO9 SychO9 removed this from the 1.1 milestone Aug 11, 2021
@dsevillamartin dsevillamartin force-pushed the ds/876-lazy-draw-dropdowns branch from db9f165 to a36a3fe Compare August 13, 2021 14:20
@askvortsov1 askvortsov1 merged commit 05121b9 into master Oct 13, 2021
@askvortsov1 askvortsov1 deleted the ds/876-lazy-draw-dropdowns branch October 13, 2021 18:55
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

Successfully merging this pull request may close these issues.

Lazy-draw some dropdown menus to improve performance
4 participants