Merge pull request #66 from dhis2/update-select

fix(select): revisit select

cypress/integration/MultiSelect/position/index.js

@@ -31,105 +31,129 @@ When('the window is scrolled down', () => {
 })
 
 When('the window is resized to a greater width', () => {
-    waitForResizeObserver(() => {
-        cy.viewport(1200, 660)
-    })
+    cy.viewport(1200, 660)
 })
 
 When('an option is clicked', () => {
-    waitForResizeObserver(() => {
-        cy.contains('option one').click()
-    })
+    cy.contains('option one').click()
 })
 
 Then('the input is empty', () => {
-    cy.get('[data-test="dhis2-uicore-select-input"]').then($el => {
-        cy.wrap($el.outerHeight()).as('emptyInputHeight')
+    cy.get('[data-test="dhis2-uicore-select-input"]').then(inputs => {
+        expect(inputs.length).to.equal(1)
+
+        const $input = inputs[0]
+        const inputRect = $input.getBoundingClientRect()
+
+        cy.wrap(inputRect.height).as('emptyInputHeight')
     })
+
     cy.get('[data-test="dhis2-uicore-select-input"] .root').should('be.empty')
 })
 
 Then('the Input grows in height', () => {
-    cy.get('@emptyInputHeight').then(emptyInputHeight => {
-        cy.get('[data-test="dhis2-uicore-select-input"]').then($el => {
-            expect($el.outerHeight()).to.be.greaterThan(emptyInputHeight)
-        })
-    })
+    const emptyInputHeight = '@emptyInputHeight'
+    const inputDataTest = '[data-test="dhis2-uicore-select-input"]'
+
+    cy.getAll(emptyInputHeight, inputDataTest).should(
+        ([emptyInputHeight, inputs]) => {
+            expect(inputs.length).to.equal(1)
+
+            const $input = inputs[0]
+            const inputRect = $input.getBoundingClientRect()
+
+            expect(inputRect.height).to.be.greaterThan(emptyInputHeight)
+        }
+    )
 })
 
 Then('the top of the menu is aligned with the bottom of the input', () => {
-    // This test is used by the window scroll scenario
-    // so needs to take y into account for the anchor
-    getAnchorAndMenuRects().then(([anchorRect, menuRect]) => {
-        expect(menuRect.top).to.equal(
-            anchorRect.y - anchorRect.top + anchorRect.height
-        )
+    const selectDataTest = '[data-test="dhis2-uicore-multiselect"]'
+    const menuDataTest = '[data-test="dhis2-uicore-select-menu-menuwrapper"]'
+
+    cy.getAll(selectDataTest, menuDataTest).should(([selects, menus]) => {
+        expect(selects.length).to.equal(1)
+        expect(menus.length).to.equal(1)
+
+        const $select = selects[0]
+        const $menu = menus[0]
+
+        const selectRect = $select.getBoundingClientRect()
+        const menuRect = $menu.getBoundingClientRect()
+
+        expect(menuRect.top).to.equal(selectRect.bottom)
     })
 })
 
 Then('the bottom of the menu is aligned with the top of the input', () => {
-    getAnchorAndMenuRects().then(([anchorRect, menuRect]) => {
-        expect(anchorRect.top).to.equal(menuRect.bottom)
+    const selectDataTest = '[data-test="dhis2-uicore-multiselect"]'
+    const menuDataTest = '[data-test="dhis2-uicore-select-menu-menuwrapper"]'
+
+    cy.getAll(selectDataTest, menuDataTest).should(([selects, menus]) => {
+        expect(selects.length).to.equal(1)
+        expect(menus.length).to.equal(1)
+
+        const $select = selects[0]
+        const $menu = menus[0]
+
+        const selectRect = $select.getBoundingClientRect()
+        const menuRect = $menu.getBoundingClientRect()
+
+        expect(selectRect.top).to.equal(menuRect.bottom)
     })
 })
 
 Then('it is rendered on top of the MultiSelect', () => {
-    getAnchorAndMenuRects().then(([anchorRect, menuRect]) => {
-        expect(anchorRect.top).to.be.greaterThan(menuRect.top)
-        expect(menuRect.bottom).to.be.greaterThan(anchorRect.bottom)
+    const selectDataTest = '[data-test="dhis2-uicore-multiselect"]'
+    const menuDataTest = '[data-test="dhis2-uicore-select-menu-menuwrapper"]'
+
+    cy.getAll(selectDataTest, menuDataTest).should(([selects, menus]) => {
+        expect(selects.length).to.equal(1)
+        expect(menus.length).to.equal(1)
+
+        const $select = selects[0]
+        const $menu = menus[0]
+
+        const selectRect = $select.getBoundingClientRect()
+        const menuRect = $menu.getBoundingClientRect()
+
+        expect(selectRect.top).to.be.greaterThan(menuRect.top)
+        expect(menuRect.bottom).to.be.greaterThan(selectRect.bottom)
     })
 })
 
 Then('the left of the Menu is aligned with the left of the Input', () => {
-    getAnchorAndMenuRects().then(([anchorRect, menuRect]) => {
-        expect(anchorRect.left).to.equal(menuRect.left)
-    })
-})
+    const selectDataTest = '[data-test="dhis2-uicore-multiselect"]'
+    const menuDataTest = '[data-test="dhis2-uicore-select-menu-menuwrapper"]'
 
-Then('the Menu and the Input have an equal width', () => {
-    cy.get('[data-test="dhis2-uicore-multiselect"] .root-input').then(
-        $input => {
-            cy.get('[data-test="dhis2-uicore-select-menu-menuwrapper"]').then(
-                $menu => {
-                    expect($input.outerWidth()).to.equal($menu.outerWidth())
-                }
-            )
-        }
-    )
-})
+    cy.getAll(selectDataTest, menuDataTest).should(([selects, menus]) => {
+        expect(selects.length).to.equal(1)
+        expect(menus.length).to.equal(1)
 
-function getAnchorAndMenuRects() {
-    return cy.getPositionsBySelectors(
-        '[data-test="dhis2-uicore-multiselect"]',
-        '[data-test="dhis2-uicore-select-menu-menuwrapper"]'
-    )
-}
+        const $select = selects[0]
+        const $menu = menus[0]
 
-function waitForResizeObserver(callback) {
-    cy.window().then(() => {
-        const id = 'resize-observer-callback-executed'
-        const oldNode = document.getElementById(id)
+        const selectRect = $select.getBoundingClientRect()
+        const menuRect = $menu.getBoundingClientRect()
 
-        // Cleanup
-        if (oldNode) {
-            oldNode.parentNode.removeChild(oldNode)
-        }
+        expect(selectRect.left).to.equal(menuRect.left)
+    })
+})
+
+Then('the Menu and the Input have an equal width', () => {
+    const inputDataTest = '[data-test="dhis2-uicore-multiselect"] .root-input'
+    const menuDataTest = '[data-test="dhis2-uicore-select-menu-menuwrapper"]'
 
-        cy.get('[data-test="dhis2-uicore-select"]').then($el => {
-            const el = $el[0]
-            const observer = new ResizeObserver(() => {
-                // Create element to wait for when resizeObserver callback is executed
-                const newNode = document.createElement('div')
-                newNode.setAttribute('id', id)
-                el.parentNode.appendChild(newNode)
-            })
+    cy.getAll(inputDataTest, menuDataTest).should(([inputs, menus]) => {
+        expect(inputs.length).to.equal(1)
+        expect(menus.length).to.equal(1)
 
-            observer.observe(el)
+        const $input = inputs[0]
+        const $menu = menus[0]
 
-            callback()
+        const inputRect = $input.getBoundingClientRect()
+        const menuRect = $menu.getBoundingClientRect()
 
-            // Wait for element and DOM redraw
-            return cy.get(`#${id}`).then(() => cy.wait(1))
-        })
+        expect(inputRect.width).to.equal(menuRect.width)
     })
-}
+})

cypress/integration/SingleSelect/position/index.js

@@ -30,38 +30,78 @@ When('the window is scrolled down', () => {
 })
 
 Then('the top of the menu is aligned with the bottom of the input', () => {
-    // This test is used by the window scroll scenario
-    // so needs to take y into account for the anchor
-    getAnchorAndMenuRects().then(([anchorRect, menuRect]) => {
-        expect(menuRect.top).to.equal(anchorRect.bottom - anchorRect.y)
+    const selectDataTest = '[data-test="dhis2-uicore-singleselect"]'
+    const menuDataTest = '[data-test="dhis2-uicore-select-menu-menuwrapper"]'
+
+    cy.getAll(selectDataTest, menuDataTest).should(([selects, menus]) => {
+        expect(selects.length).to.equal(1)
+        expect(menus.length).to.equal(1)
+
+        const $select = selects[0]
+        const $menu = menus[0]
+
+        const selectRect = $select.getBoundingClientRect()
+        const menuRect = $menu.getBoundingClientRect()
+
+        expect(menuRect.top).to.equal(selectRect.bottom)
     })
 })
 
 Then('the bottom of the menu is aligned with the top of the input', () => {
-    getAnchorAndMenuRects().then(([anchorRect, menuRect]) => {
-        expect(anchorRect.top).to.equal(menuRect.bottom)
+    const selectDataTest = '[data-test="dhis2-uicore-singleselect"]'
+    const menuDataTest = '[data-test="dhis2-uicore-select-menu-menuwrapper"]'
+
+    cy.getAll(selectDataTest, menuDataTest).should(([selects, menus]) => {
+        expect(selects.length).to.equal(1)
+        expect(menus.length).to.equal(1)
+
+        const $select = selects[0]
+        const $menu = menus[0]
+
+        const selectRect = $select.getBoundingClientRect()
+        const menuRect = $menu.getBoundingClientRect()
+
+        expect(selectRect.top).to.equal(menuRect.bottom)
     })
 })
 
 Then('it is rendered on top of the SingleSelect', () => {
-    getAnchorAndMenuRects().then(([anchorRect, menuRect]) => {
-        expect(anchorRect.top).to.be.greaterThan(menuRect.top)
-        expect(menuRect.bottom).to.be.greaterThan(anchorRect.bottom)
+    const selectDataTest = '[data-test="dhis2-uicore-singleselect"]'
+    const menuDataTest = '[data-test="dhis2-uicore-select-menu-menuwrapper"]'
+
+    cy.getAll(selectDataTest, menuDataTest).should(([selects, menus]) => {
+        expect(selects.length).to.equal(1)
+        expect(menus.length).to.equal(1)
+
+        const $select = selects[0]
+        const $menu = menus[0]
+
+        const selectRect = $select.getBoundingClientRect()
+        const menuRect = $menu.getBoundingClientRect()
+
+        expect(selectRect.top).to.be.greaterThan(menuRect.top)
+        expect(menuRect.bottom).to.be.greaterThan(selectRect.bottom)
     })
 })
 
 Then(
     'the left of the SingleSelect is aligned with the left of the anchor',
     () => {
-        getAnchorAndMenuRects().then(([anchorRect, menuRect]) => {
-            expect(anchorRect.left).to.equal(menuRect.left)
+        const selectDataTest = '[data-test="dhis2-uicore-singleselect"]'
+        const menuDataTest =
+            '[data-test="dhis2-uicore-select-menu-menuwrapper"]'
+
+        cy.getAll(selectDataTest, menuDataTest).should(([selects, menus]) => {
+            expect(selects.length).to.equal(1)
+            expect(menus.length).to.equal(1)
+
+            const $select = selects[0]
+            const $menu = menus[0]
+
+            const selectRect = $select.getBoundingClientRect()
+            const menuRect = $menu.getBoundingClientRect()
+
+            expect(selectRect.left).to.equal(menuRect.left)
         })
     }
 )
-
-function getAnchorAndMenuRects() {
-    return cy.getPositionsBySelectors(
-        '[data-test="dhis2-uicore-singleselect"]',
-        '[data-test="dhis2-uicore-select-menu-menuwrapper"]'
-    )
-}

cypress/support/getAll.js

@@ -0,0 +1,9 @@
+Cypress.Commands.add('getAll', (...elements) => {
+    const promise = cy.wrap([], { log: false })
+
+    for (const element of elements) {
+        promise.then(arr => cy.get(element).then(got => cy.wrap([...arr, got])))
+    }
+
+    return promise
+})

cypress/support/index.js

@@ -5,6 +5,7 @@ import './all'
 import './clickWith'
 import './find'
 import './get'
+import './getAll'
 import './getPositionsBySelectors'
 import './uploadMultipleFiles'
 import './uploadSingleFile'

packages/core/src/Select/MenuWrapper.js

@@ -22,7 +22,7 @@ const MenuWrapper = ({
         <Layer onClick={onClick} transparent>
             <Popper
                 reference={selectRef}
-                placement="bottom"
+                placement="bottom-start"
                 observeReferenceResize
             >
                 <div data-test={`${dataTest}-menuwrapper`}>

packages/core/src/Select/Select.js

@@ -3,6 +3,7 @@ import propTypes from '@dhis2/prop-types'
 import { sharedPropTypes } from '@dhis2/ui-constants'
 import { InputWrapper } from './InputWrapper.js'
 import { MenuWrapper } from './MenuWrapper.js'
+import { debounce } from './debounce'
 
 // Keycodes for the keypress event handlers
 const ESCAPE_KEY = 27
@@ -32,15 +33,24 @@ export class Select extends Component {
         window.removeEventListener('resize', this.setMenuWidth)
     }
 
-    setMenuWidth = () => {
+    /**
+     * We're debouncing this so it doesn't fire continually during a resize.
+     *
+     * Additionally we should use requestPostAnimationFrame to not trigger a forced
+     * layout, but that's just a proposal, and the added complexity of solving this
+     * in another manner does not seem worth it, considering the minor perf penalty.
+     *
+     * See: https://nolanlawson.com/2018/09/25/accurately-measuring-layout-on-the-web
+     */
+    setMenuWidth = debounce(() => {
         const inputWidth = `${this.inputRef.current.offsetWidth}px`
 
         if (this.state.menuWidth !== inputWidth) {
             this.setState({
                 menuWidth: inputWidth,
             })
         }
-    }
+    }, 50)
 
     handleFocusInput = () => {
         this.inputRef.current.focus()

packages/core/src/Select/debounce/__tests__/index.test.js

@@ -0,0 +1,35 @@
+import { debounce } from '../index.js'
+
+beforeEach(() => {
+    jest.useFakeTimers()
+})
+
+describe('debounce', () => {
+    it('should call the debounced function once after the timeout without immediate', () => {
+        const spy = jest.fn()
+        const debounced = debounce(spy, 100)
+
+        debounced()
+        debounced()
+
+        expect(spy).not.toHaveBeenCalled()
+
+        jest.runAllTimers()
+
+        expect(spy).toHaveBeenCalledTimes(1)
+    })
+
+    it('should call the debounced function once immediately if immediate is set', () => {
+        const spy = jest.fn()
+        const debounced = debounce(spy, 100, true)
+
+        debounced()
+        debounced()
+
+        expect(spy).toHaveBeenCalledTimes(1)
+
+        jest.runAllTimers()
+
+        expect(spy).toHaveBeenCalledTimes(1)
+    })
+})