Skip to content

Commit 702c923

Browse files
Chris Hartjunyper
authored andcommitted
fix(ui-utils,getFontSize): Improve performance
fixes: INSTUI-1012 Updates getFontSize to add caching of the fontSize and use browser-computed style instead of generating an element and getting the line-height. Test plan: - Run `yarn test:watch --scope @instructure/ui-utils` - Play with the em and rem tests for px to see if you can break them. I left the getFontSize() calls in the px tests so we can compare the new method of calculating fontSize to the one we've been using. Change-Id: I47e5fad2360e4a0d5aa211a9642fbca186c8ebec Reviewed-on: https://gerrit.instructure.com/147342 Reviewed-by: Jennifer Stern <jstern@instructure.com> Product-Review: Jennifer Stern <jstern@instructure.com> Tested-by: Jenkins QA-Review: Dan Sasaki <dsasaki@instructure.com>
1 parent 5054f9d commit 702c923

File tree

2 files changed

+35
-28
lines changed

2 files changed

+35
-28
lines changed

packages/ui-overlays/src/components/Popover/index.js

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,9 @@ class Popover extends Component {
297297
super(props)
298298

299299
this.state = {
300-
placement: props.placement
300+
placement: props.placement,
301+
offsetX: props.offsetX,
302+
offsetY: props.offsetY
301303
}
302304

303305
if (props.show === undefined) {
@@ -370,7 +372,8 @@ class Popover extends Component {
370372
handlePositionChanged = ({ placement }) => {
371373
this.setState({
372374
closeButtonPlacement: this.getCloseButtonPlacement(this.props),
373-
placement
375+
placement,
376+
...this.computeOffsets(placement)
374377
})
375378
}
376379

@@ -494,11 +497,11 @@ class Popover extends Component {
494497
}
495498
}
496499

497-
get positionProps () {
498-
let offsetX = this.props.offsetX
499-
let offsetY = this.props.offsetY
500+
computeOffsets (placement) {
501+
let { offsetX, offsetY } = this.props
502+
500503
if (this.props.alignArrow && this._contextBox) {
501-
const secondaryPlacement = parsePlacement(this.state.placement)[1]
504+
const secondaryPlacement = parsePlacement(placement)[1]
502505
const { arrowSize, borderWidth } = this._contextBox.theme
503506
const offsetAmount = (px(arrowSize) + px(borderWidth)) * 2
504507
if (secondaryPlacement === 'start') {
@@ -513,9 +516,16 @@ class Popover extends Component {
513516
}
514517

515518
return {
516-
...pickProps(this.props, Position.propTypes),
517519
offsetX,
518-
offsetY,
520+
offsetY
521+
}
522+
}
523+
524+
get positionProps () {
525+
return {
526+
...pickProps(this.props, Position.propTypes),
527+
offsetX: this.state.offsetX,
528+
offsetY: this.state.offsetY,
519529
trackPosition: this.shown,
520530
placement: this.placement,
521531
onPositioned: createChainedFunction(this.handlePositionChanged, this.props.onShow),

packages/ui-utils/src/dom/getFontSize.js

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@
2121
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
2222
* SOFTWARE.
2323
*/
24+
import canUseDOM from './canUseDOM'
25+
import ownerDocument from './ownerDocument'
26+
import getComputedStyle from './getComputedStyle'
27+
28+
const COMPUTED_CACHE = {}
2429

2530
/**
2631
* ---
@@ -32,30 +37,22 @@
3237
* @param {ReactComponent|DomNode} el - component or DOM node
3338
* @returns {Object} font size in px
3439
*/
35-
export default function getFontSize (el) {
36-
const m = document.createElement('div')
40+
export default function getFontSize (el, ignoreCache) {
41+
if (!canUseDOM) {
42+
return 16
43+
}
3744

38-
let container = el || document.body
39-
let fontSize = 16
45+
const container = el || ownerDocument(el).documentElement
4046

41-
if (!container) {
42-
container = document.createElement('body')
43-
container.style.cssText = 'font-size:1em !important'
44-
document.documentElement.insertBefore(container, document.body)
47+
// return the cached font size if it's there
48+
if (!ignoreCache && COMPUTED_CACHE[container]) {
49+
return COMPUTED_CACHE[container]
4550
}
4651

47-
m.style.cssText = [
48-
'display: inline-block !important;',
49-
'padding: 0 !important;',
50-
'line-height: 1 !important;',
51-
'position: absolute !important;',
52-
'visibility: hidden !important;',
53-
'font-size: 1em !important;'
54-
].join('')
55-
m.appendChild(document.createTextNode('M'))
56-
container.appendChild(m)
57-
fontSize = m.offsetHeight
58-
container.removeChild(m)
52+
const fontSize = parseInt(getComputedStyle(container).getPropertyValue('font-size'))
53+
54+
// cache the computed font size so that we don't have to compute it again
55+
COMPUTED_CACHE[container] = fontSize
5956

6057
return fontSize
6158
}

0 commit comments

Comments
 (0)