Skip to content

Commit

Permalink
fix(ui-layout): position should account for documentElement offset
Browse files Browse the repository at this point in the history
refs: INSTUI-2046

This change fixes an issue with the Position
component noticed when the html element has an
offsetTop. This often occurs due to the no-scroll
library currently in use with our Mask component.

TEST PLAN:
- create a modal that contains a select
- ensure select is positioned correctly
- ensure all position examples render correctly
- there should be no regressions to any component
using Position
- unit tests should pass

Change-Id: Id93286019acd2e4b3e67e073453e11e4e375d2e7
Reviewed-on: https://gerrit.instructure.com/190531
Tested-by: Jenkins
Reviewed-by: Jennifer Stern <jstern@instructure.com>
Product-Review: Jennifer Stern <jstern@instructure.com>
QA-Review: Omar Khan <okhan@instructure.com>
Visual-Regression-Test: Daniel Sasaki <dsasaki@instructure.com>
Reviewed-on: https://gerrit.instructure.com/190883
Reviewed-by: Steve Jensen <sejensen@instructure.com>
QA-Review: Steve Jensen <sejensen@instructure.com>
Product-Review: Steve Jensen <sejensen@instructure.com>
Visual-Regression-Test: Chris Hart <chart@instructure.com>
  • Loading branch information
kmeleta authored and Chris Hart committed Apr 25, 2019
1 parent f575090 commit f70bde6
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ export default {

return {
constrain: 'none',
trackPosition: false,
insertAt: 'bottom',
over: xStretch || yStretch,
offsetX: yStretch ? -parseInt(targetSize) : 0,
offsetY: xStretch ? -parseInt(targetSize) : 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -548,4 +548,68 @@ describe('<Position />', async () => {
expect(within(Math.floor(contentRect.right), Math.floor(targetRect.left), 1)).to.be.true()
})
})

describe('when the documentElement is offset', async () => {
beforeEach(async () => {
document.documentElement.style.position = 'fixed'
document.documentElement.style.top = '-100px'
})

it('should position correctly', async () => {
await mount(
<div style={{padding: '100px'}}>
<Position placement="bottom">
<PositionTarget>
<button>Target</button>
</PositionTarget>
<PositionContent>
<div id="content">
<div>Content</div>
</div>
</PositionContent>
</Position>
</div>
)

const position = await PositionLocator.find()
const target = await position.findTarget(':contains(Target)')
const content = await position.findContent(':contains(Content)')

await wait(() => {
expect(content.getBoundingClientRect().top).to.equal(target.getBoundingClientRect().bottom)
})
})

it('should position correctly with mountNode', async () => {
await mount(
<div style={{padding: '100px'}}>
<div id="mountNode">mount</div>
<div style={{width: '50px', height: '50px', overflow: 'scroll', padding: '50px'}}>
<Position
placement="bottom"
constrain="scroll-parent"
mountNode={el => document.getElementById('mountNode')}
>
<PositionTarget>
<button>Target</button>
</PositionTarget>
<PositionContent>
<div id="content">
<div>Content</div>
</div>
</PositionContent>
</Position>
</div>
</div>
)

const position = await PositionLocator.find()
const target = await position.findTarget(':contains(Target)')
const content = await position.findContent(':contains(Content)')

await wait(() => {
expect(content.getBoundingClientRect().top).to.equal(target.getBoundingClientRect().bottom)
})
})
})
})
8 changes: 6 additions & 2 deletions packages/ui-layout/src/utils/calculateElementPosition.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,12 @@ class PositionedElement {
// ancestor. We calculate the offset between the child and
// positioned parent so we can negate that distance
const parents = getOffsetParents(this.node)
const doc = ownerDocument(this.node)

let offsetY = 0
// If there is more than one parent, the offset on the
// documentElement should be calculated appropriately.
// Otherwise we need to explictly account for that offset
let offsetY = parents.length > 1 ? 0 : doc.documentElement.offsetTop
let offsetX = 0
let scrollY = 0

Expand All @@ -202,7 +206,7 @@ class PositionedElement {
offsetY = offsetY + (child.top - parent.top)
offsetX = offsetX + (child.left - parent.left)

if (parents[i] === ownerDocument(this.node).body) {
if (parents[i] === doc.body) {
// accounts for any margin on body
offsetY = offsetY + parent.top
offsetX = offsetX + parent.left
Expand Down

0 comments on commit f70bde6

Please sign in to comment.