Skip to content

Commit

Permalink
fix(Drawer): ensure ScrollView is used for scrollable content (#1632)
Browse files Browse the repository at this point in the history
  • Loading branch information
tujoworker authored Oct 14, 2022
1 parent 7ed9550 commit fea2358
Show file tree
Hide file tree
Showing 29 changed files with 216 additions and 85 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ SimpleDrawerExample,
FullDrawerExample,
DrawerCallbackExample,
DrawerCustomTriggerExample,
DrawerNoAnimationNoSpacing
DrawerNoAnimationNoSpacing,
} from 'Docs/uilib/components/drawer/Examples'

## Demos
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/**
* UI lib Component Example
*
*/

import React from 'react'
import ComponentBox from 'dnb-design-system-portal/src/shared/tags/ComponentBox'

export const DrawerScrollViewSetup = () => (
<ComponentBox useRender data-visual-test="drawer-scroll-view">
{
/* jsx */ `
const DrawerMock = () => {
const scrollRef = React.useRef()
const innerRef = React.useRef()
const [errorMessage, setErrorMessage] = React.useState(null)
const message = errorMessage
? errorMessage
: 'Yes, the "dnb-scroll-view" is used!'
return (
<Drawer
contentRef={innerRef}
scrollRef={scrollRef}
onOpen={() => {
const innerOverflowY = window.getComputedStyle(
innerRef.current
).overflowY
const contentElem = scrollRef.current.querySelector(
'.dnb-drawer__content'
)
const contentOverflowY =
window.getComputedStyle(contentElem)?.overflowY
const scxrollOverflowY = window.getComputedStyle(
scrollRef.current
).overflowY
if (contentOverflowY !== 'visible') {
setErrorMessage(
'.dnb-drawer__content was "'+contentOverflowY+'" and not "visible"'
)
} else if (innerOverflowY !== 'visible') {
setErrorMessage(
'.dnb-drawer__inner was "'+innerOverflowY+'" and not "visible"'
)
} else if (scxrollOverflowY !== 'auto') {
setErrorMessage('.dnb-scroll-view was not "auto"')
}
}}
>
<Drawer.Body>
<div style={{ height: '100rem' }}>
<div className="drawer-scroll-view">
<P size="x-large">{message}</P>
</div>
</div>
</Drawer.Body>
</Drawer>
)
}
render(<DrawerMock />)
`
}
</ComponentBox>
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
draft: true
---

import {
DrawerScrollViewSetup,
} from 'Docs/uilib/components/drawer/visual-tests/Examples'

<DrawerScrollViewSetup />
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
attachToBody,
} from '../../../core/jest/jestSetup'
import * as helpers from '../../../shared/helpers'
import { render } from '@testing-library/react'

const props = fakeProps(require.resolve('../Dialog.tsx'), {
all: true,
Expand Down Expand Up @@ -93,6 +94,29 @@ describe('Dialog', () => {
expect(on_close).toHaveBeenCalledTimes(1)
})

it('will accept custom refs', () => {
const contentRef = React.createRef<HTMLElement>()
const scrollRef = React.createRef<HTMLElement>()

const MockComponent = () => {
return (
<Dialog
openState
noAnimation
contentRef={contentRef}
scrollRef={scrollRef}
>
content
</Dialog>
)
}

render(<MockComponent />)

expect(contentRef.current).toBeTruthy()
expect(scrollRef.current).toBeTruthy()
})

it('will use props from global context', () => {
const contextTitle = 'Custom title'
const Comp = mount(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1354,7 +1354,8 @@ exports[`Dialog component snapshot should match component snapshot 1`] = `
`;

exports[`Dialog scss have to match snapshot 1`] = `
"/*
"@charset \\"UTF-8\\";
/*
* DNB Drawer
*
*/
Expand Down Expand Up @@ -2153,11 +2154,15 @@ button.dnb-button::-moz-focus-inner {
display: flex;
flex-direction: column;
z-index: 10;
/** Sets the color on scroll overflow (at the bottom) */
background-color: var(--modal-background-color, transparent);
/**
NB: Do not use \\"overflow-x: hidden;\\" in here,
(overflowing one direction, will influence the other – we can't just have one at a time)
To make the Drawer scroll, we use .dnb-scroll-view
*/
width: 100%;
height: 100%;
overflow-x: hidden; }
/** Sets the color on scroll overflow (at the bottom) */
background-color: var(--modal-background-color, transparent); }
.dnb-drawer--spacing .dnb-drawer__inner {
padding: 0 var(--drawer-spacing) 0; }
@media screen and (min-width: 72em) {
Expand Down Expand Up @@ -2264,8 +2269,8 @@ button.dnb-button::-moz-focus-inner {
position: sticky;
top: 0;
z-index: 99;
margin: var(--drawer-spacing) calc(var(--drawer-spacing) * -1);
padding: 0 var(--drawer-spacing); }
margin: var(--drawer-spacing) calc(var(--drawer-spacing) * -1 * 1.75);
padding: 0 calc(var(--drawer-spacing) * 1.5); }
.dnb-drawer--spacing .dnb-drawer__navigation.dnb-section.dnb-drawer__navigation--sticky {
z-index: 2999; }
.dnb-drawer .dnb-drawer__navigation--sticky::before {
Expand Down
4 changes: 4 additions & 0 deletions packages/dnb-eufemia/src/components/drawer/Drawer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ function Drawer({
triggerAttributes,
overlayClass,
contentClass,
contentRef,
scrollRef,

top,
bottom,
Expand Down Expand Up @@ -92,6 +94,8 @@ function Drawer({
trigger,
triggerAttributes,
overlayClass,
contentRef,
scrollRef,
top,
bottom,
left,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,22 @@ describe('Drawer no animation/spacing screenshot', () => {
expect(screenshot).toMatchImageSnapshot()
})
})

describe('Drawer screenshot', () => {
setupPageScreenshot({
url: '/uilib/components/drawer/visual-tests/hidden-tests',
pageViewport,
})

it('have to match correct scroll view setup', async () => {
const screenshot = await testPageScreenshot({
selector: '[data-visual-test="drawer-scroll-view"]',
simulate: 'click',
simulateSelector:
'[data-visual-test="drawer-scroll-view"] button:first-of-type',
screenshotSelector: '.drawer-scroll-view',
rootClassName: 'hide-page-content',
})
expect(screenshot).toMatchImageSnapshot()
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
axeComponent,
attachToBody,
} from '../../../core/jest/jestSetup'
import { render } from '@testing-library/react'

const props = fakeProps(require.resolve('../Drawer.tsx'), {
all: true,
Expand Down Expand Up @@ -283,6 +284,29 @@ describe('Drawer', () => {
).toBe(false)
})

it('will accept custom refs', () => {
const contentRef = React.createRef<HTMLElement>()
const scrollRef = React.createRef<HTMLElement>()

const MockComponent = () => {
return (
<Drawer
openState
noAnimation
contentRef={contentRef}
scrollRef={scrollRef}
>
content
</Drawer>
)
}

render(<MockComponent />)

expect(contentRef.current).toBeTruthy()
expect(scrollRef.current).toBeTruthy()
})

it('can contain drawer parts', () => {
const Comp = mount(
<Drawer noAnimation directDomReturn={false}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1369,7 +1369,8 @@ exports[`Drawer component snapshot should match component snapshot 1`] = `
`;

exports[`Drawer scss have to match snapshot 1`] = `
"/*
"@charset \\"UTF-8\\";
/*
* DNB Drawer
*
*/
Expand Down Expand Up @@ -2168,11 +2169,15 @@ button.dnb-button::-moz-focus-inner {
display: flex;
flex-direction: column;
z-index: 10;
/** Sets the color on scroll overflow (at the bottom) */
background-color: var(--modal-background-color, transparent);
/**
NB: Do not use \\"overflow-x: hidden;\\" in here,
(overflowing one direction, will influence the other – we can't just have one at a time)
To make the Drawer scroll, we use .dnb-scroll-view
*/
width: 100%;
height: 100%;
overflow-x: hidden; }
/** Sets the color on scroll overflow (at the bottom) */
background-color: var(--modal-background-color, transparent); }
.dnb-drawer--spacing .dnb-drawer__inner {
padding: 0 var(--drawer-spacing) 0; }
@media screen and (min-width: 72em) {
Expand Down Expand Up @@ -2279,8 +2284,8 @@ button.dnb-button::-moz-focus-inner {
position: sticky;
top: 0;
z-index: 99;
margin: var(--drawer-spacing) calc(var(--drawer-spacing) * -1);
padding: 0 var(--drawer-spacing); }
margin: var(--drawer-spacing) calc(var(--drawer-spacing) * -1 * 1.75);
padding: 0 calc(var(--drawer-spacing) * 1.5); }
.dnb-drawer--spacing .dnb-drawer__navigation.dnb-section.dnb-drawer__navigation--sticky {
z-index: 2999; }
.dnb-drawer .dnb-drawer__navigation--sticky::before {
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
16 changes: 10 additions & 6 deletions packages/dnb-eufemia/src/components/drawer/style/_drawer.scss
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
}

@include defaultDropShadow();
//border-radius: 0.5rem;
user-select: text;
-webkit-user-select: text; // Safari / Touch fix
border: none;
Expand All @@ -46,12 +45,17 @@
flex-direction: column;
z-index: 10;

/** Sets the color on scroll overflow (at the bottom) */
background-color: var(--modal-background-color, transparent);
/**
NB: Do not use "overflow-x: hidden;" in here,
(overflowing one direction, will influence the other – we can't just have one at a time)
To make the Drawer scroll, we use .dnb-scroll-view
*/

width: 100%;
height: 100%;
overflow-x: hidden;

/** Sets the color on scroll overflow (at the bottom) */
background-color: var(--modal-background-color, transparent);
}

&--spacing &__inner {
Expand Down Expand Up @@ -218,8 +222,8 @@
z-index: 99; // below #dropdown and #date-picker

// on large screens
margin: var(--drawer-spacing) calc(var(--drawer-spacing) * -1);
padding: 0 var(--drawer-spacing);
margin: var(--drawer-spacing) calc(var(--drawer-spacing) * -1 * 1.75);
padding: 0 calc(var(--drawer-spacing) * 1.5);
}
&--spacing &__navigation.dnb-section#{&}__navigation--sticky {
z-index: 2999; // above #dropdown and #date-picker and below #modal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,26 +63,6 @@ describe('Modal screenshot', () => {
})
})

describe('Drawer screenshot', () => {
setupPageScreenshot({
url: '/uilib/components/modal/visual-tests/hidden-tests',
pageViewport,
})

it('have to match the default drawer window', async () => {
const screenshot = await testPageScreenshot({
selector: 'div#dnb-modal-root', // only to make sure we have a valid selector
simulate: 'click',
simulateSelector:
'[data-visual-test="modal-drawer-basic"] button:first-of-type',
screenshotSelector: '.dnb-modal__content',
rootClassName: 'hide-page-content',
waitBeforeSimulate: 200,
})
expect(screenshot).toMatchImageSnapshot()
})
})

describe('Additional Modal screenshot', () => {
const pageViewport = {
width: 400,
Expand All @@ -105,42 +85,3 @@ describe('Additional Modal screenshot', () => {
expect(screenshot).toMatchImageSnapshot()
})
})

describe('Drawer without spacing', () => {
setupPageScreenshot({
url: '/uilib/components/modal/visual-tests/hidden-tests',
pageViewport,
})

it('have to match drawer without spacing', async () => {
const screenshot = await testPageScreenshot({
selector: 'div#dnb-modal-root', // only to make sure we have a valid selector
simulate: 'click',
simulateSelector:
'[data-visual-test="drawer-no-spacing"] button:first-of-type',
screenshotSelector: '.dnb-modal__content',
rootClassName: 'hide-page-content',
waitBeforeSimulate: 200,
})
expect(screenshot).toMatchImageSnapshot()
})
})

describe('Drawer left screenshot', () => {
setupPageScreenshot({
url: '/uilib/components/modal/visual-tests/hidden-tests',
pageViewport,
})

it('have to match the drawer with containerplacement left', async () => {
const screenshot = await testPageScreenshot({
selector: 'div#dnb-modal-root', // only to make sure we have a valid selector
simulate: 'click',
simulateSelector:
'[data-visual-test="modal-drawer-leftsided"] button:first-of-type',
screenshotSelector: '.dnb-modal__content',
rootClassName: 'hide-page-content',
})
expect(screenshot).toMatchImageSnapshot()
})
})
Loading

0 comments on commit fea2358

Please sign in to comment.