Skip to content

Comments

perf(frontend): lazy load the unified search modal #43400

Closed
st3iny wants to merge 1 commit intomasterfrom
chore/perf/lazy-search
Closed

perf(frontend): lazy load the unified search modal #43400
st3iny wants to merge 1 commit intomasterfrom
chore/perf/lazy-search

Conversation

@st3iny
Copy link
Member

@st3iny st3iny commented Feb 6, 2024

  • Resolves: #

Summary

Defer loading the unified search modal until it is first opened. Show a loading spinner while the modal is being loaded in the background.

(Bandwidth has been throttled during the recording to exaggerate the effect.)

Screencast_20240207_112434-1.mp4

Checklist

@st3iny st3iny requested review from a team, ShGKme, nfebe and susnux February 6, 2024 18:06
@st3iny st3iny self-assigned this Feb 6, 2024
@st3iny st3iny requested review from sorbaugh and removed request for a team February 6, 2024 18:06
@st3iny st3iny force-pushed the chore/perf/lazy-search branch from e39bfba to 1435eab Compare February 6, 2024 18:08
@st3iny st3iny changed the title chore(perf): lazy load the unified search modal perf(frontend): lazy load the unified search modal Feb 6, 2024
@st3iny st3iny removed the request for review from sorbaugh February 6, 2024 18:09
Copy link
Contributor

@nfebe nfebe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test and it works, please address earlier comments.

Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather recommend not lazy load the unified search modal but show a modal with NcEmptyContent and loading spinner and just lazy load the modal content.

@st3iny st3iny force-pushed the chore/perf/lazy-search branch from 1435eab to 020b5e2 Compare February 7, 2024 10:29
@st3iny
Copy link
Member Author

st3iny commented Feb 7, 2024

I implemented a loading spinner and added a screencast to the description. Note that I throttled my bandwidth during the recording to exaggerate the effect. It loads much faster on a regular connection.

@st3iny st3iny requested review from ShGKme, nfebe and susnux February 7, 2024 10:30
@st3iny st3iny force-pushed the chore/perf/lazy-search branch from 020b5e2 to 4c7974e Compare February 7, 2024 10:34
Copy link
Contributor

@nfebe nfebe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not tested, code looks okay

Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
@st3iny st3iny force-pushed the chore/perf/lazy-search branch from 4c7974e to 949044b Compare February 7, 2024 13:12
@st3iny st3iny enabled auto-merge February 7, 2024 13:12
@st3iny
Copy link
Member Author

st3iny commented Mar 6, 2024

I'm closing this one as the benefit is too small.

grafik

@st3iny st3iny closed this Mar 6, 2024
auto-merge was automatically disabled March 6, 2024 13:20

Pull request was closed

@st3iny st3iny deleted the chore/perf/lazy-search branch March 6, 2024 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants