-
Notifications
You must be signed in to change notification settings - Fork 51
Tag - Fix performance issues with single ResizeObserver #3033
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
Changes from all commits
d76f27a
3b2102e
9839faa
32ec80f
efe6bb1
1a18adb
c9b537c
1b4c0be
2b5b836
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
--- | ||
"@hashicorp/design-system-components": patch | ||
--- | ||
|
||
<!-- START components/tag --> | ||
`Tag` - Fixed a performance issue when many tags are present on a page caused by the ResizeObserver | ||
<!-- END --> | ||
|
||
Dependencies - Added `tracked-built-ins` |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -5,6 +5,7 @@ | |||||
|
||||||
import Component from '@glimmer/component'; | ||||||
import { tracked } from '@glimmer/tracking'; | ||||||
import { TrackedWeakSet } from 'tracked-built-ins'; | ||||||
import { assert } from '@ember/debug'; | ||||||
import { modifier } from 'ember-modifier'; | ||||||
|
||||||
|
@@ -33,23 +34,42 @@ export interface HdsTagSignature { | |||||
Element: HTMLSpanElement; | ||||||
} | ||||||
|
||||||
const overflowed = new TrackedWeakSet<Element>(); | ||||||
|
||||||
const observer = new ResizeObserver((entries) => { | ||||||
entries.forEach((entry) => { | ||||||
const textContainer = entry.target.querySelector( | ||||||
'.hds-tag__text-container' | ||||||
); | ||||||
if ( | ||||||
textContainer && | ||||||
textContainer.scrollHeight > textContainer.clientHeight | ||||||
) { | ||||||
overflowed.add(entry.target); | ||||||
} else { | ||||||
overflowed.delete(entry.target); | ||||||
} | ||||||
}); | ||||||
}); | ||||||
|
||||||
export default class HdsTag extends Component<HdsTagSignature> { | ||||||
@tracked private _isTextOverflow!: boolean; | ||||||
private _observer!: ResizeObserver; | ||||||
@tracked private _element?: HTMLElement; | ||||||
private get _isTextOverflow(): boolean { | ||||||
if (!this._element) { | ||||||
return false; | ||||||
} | ||||||
return overflowed.has(this._element); | ||||||
} | ||||||
|
||||||
private _setUpObserver = modifier((element: HTMLElement) => { | ||||||
// Used to detect when text is clipped to one line, and tooltip should be added | ||||||
this._observer = new ResizeObserver((entries) => { | ||||||
entries.forEach((entry) => { | ||||||
this._isTextOverflow = this._isOverflow( | ||||||
entry.target.querySelector('.hds-tag__text-container')! | ||||||
); | ||||||
}); | ||||||
}); | ||||||
this._observer.observe(element); | ||||||
this._element = element; | ||||||
observer.observe(element); | ||||||
|
||||||
return () => { | ||||||
this._observer.disconnect(); | ||||||
if (this._element) { | ||||||
observer.unobserve(this._element); | ||||||
} | ||||||
delete this._element; | ||||||
dchyun marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
}; | ||||||
}); | ||||||
|
||||||
|
@@ -150,8 +170,4 @@ export default class HdsTag extends Component<HdsTagSignature> { | |||||
|
||||||
return classes.join(' '); | ||||||
} | ||||||
|
||||||
private _isOverflow(el: Element): boolean { | ||||||
return el.scrollHeight > el.clientHeight; | ||||||
} | ||||||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
/** | ||
* Copyright (c) HashiCorp, Inc. | ||
* SPDX-License-Identifier: MPL-2.0 | ||
*/ | ||
|
||
import Route from '@ember/routing/route'; | ||
|
||
export default class PageComponentsTagFramelessDemoObserverPerformanceRoute extends Route { | ||
model() { | ||
const DEMO_RANGE = Array(1000) | ||
.fill(1) | ||
.map((n, i) => ({ index: n + i })); | ||
return { DEMO_RANGE }; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
{{! | ||
Copyright (c) HashiCorp, Inc. | ||
SPDX-License-Identifier: MPL-2.0 | ||
}} | ||
|
||
{{page-title "Tag - Observer performance demo - Frameless"}} | ||
|
||
<div {{style padding="24px"}}> | ||
<Shw::Text::Body><strong>Note:</strong> | ||
This demo is to test the loading performance of the ResizeObserver when many Tags are rendered. There should be | ||
little increase in loading times even when many Tags with observers are present.</Shw::Text::Body> | ||
|
||
<Hds::Layout::Grid @columnWidth="150px" @gap="16"> | ||
{{#each @model.DEMO_RANGE as |i|}} | ||
<Hds::Tag @text="{{i.index}} This is a very long text that should go on multiple lines" /> | ||
{{/each}} | ||
</Hds::Layout::Grid> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the key to getting this working - externalizing the observer is only possible if we can track what elements are overflowed. To do this with ember's tracking,
tracked-built-ins
provides a tracking-entangledWeakSet
. This means that getters and other computed properties that rely on this WeakSet will recompute when items are added or removed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checked the performance times for before and after and this fix correct the issues greatly. Will add some screenshots to the PR description for visibility.