Use mixins and decorators for scroll restoration and tippy cleanup #2415
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Dependency already exists
The delegate addon creates tippy instances from mouse and touch events with a matching `event.target`. This is initially significantly cheaper than creating all instances at once. The addon keeps all created tippy instances alive until it is destroyed itself. `tippyMixin` destroys the addon instance after every render, as long as all instances are hidden. This drops some tippy instances that may have to be recreated later (e.g when the mouse moves over the trigger again), but is otherwise fairly cheap (creates one tippy instance).
The history module generates a random string (`location.key`) for every browser history entry. The names for saved positions include this key. The position is saved before a route component unmounts or before the `location.key` changes. The `scrollMixin` tires to restore the scroll position after every change of `location.key`. It only does so after the first render for which the route components `loadingSettled()` returns true. Things like `scrollToComments` should only scroll when `history.action` is not "POP".
matc-pub
commented
Apr 11, 2024
Comment on lines
+38
to
+50
| queueMicrotask(() => { | ||
| requested = false; | ||
| if (shownInstances.size) { | ||
| // Avoid randomly closing tooltips. | ||
| return; | ||
| } | ||
| // delegate from tippy.js creates tippy instances when needed, but only | ||
| // destroys them when the delegate instance is destroyed. | ||
| const current = instance?.reference ?? null; | ||
| destroyTippy(); | ||
| setupTippy({ current }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Looks like there is an edge case triggering the tippy memory leak warning. Positioning the mouse over a content action dropdown button, refreshing the page and then clicking without moving the mouse.
dessalines
approved these changes
Apr 11, 2024
SleeplessOne1917
approved these changes
Apr 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Mixins prevent repetitions across multiple components.
Without decorators the use of a mixin would roughly look like this:
With decorators:
The class decorator functions (the mixins) in this PR are compatible with
"legacy" and "2023-11" proposal-decorators versions. "legacy" transpiles to
slightly smaller javascript. (Switching between versions requires changes in
tsconfig and babelrc.)
Typescript requires two arguments for non-legacy class decorator functions. I
had to add
{ "argsIgnorePattern": "^_" }tono-unused-vars.Commits
Use tippy.js delegate addon, cleanup tippy instances from a mixin.
The delegate addon creates tippy instances from mouse and touch events with a
matching
event.target. This is initially significantly cheaper than creatingall instances at once. The addon keeps all created tippy instances alive until
it is destroyed itself.
tippyMixindestroys the addon instance after every render, as long as allinstances are hidden. This drops some tippy instances that may have to be
recreated later (e.g when the mouse moves over the trigger again), but is
otherwise fairly cheap (creates one tippy instance).
Restore scroll positions when resource loading settles.
The history module generates a random string (
location.key) for everybrowser history entry. The names for saved positions include this key.
The position is saved before a route component unmounts or before the
location.keychanges.The
scrollMixintires to restore the scroll position after everychange of
location.key. It only does so after the first render forwhich the route components
loadingSettled()returns true.Things like
scrollToCommentsshould only scroll whenhistory.actionis not "POP".