Skip to content
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

fix(Drawer): ensure ScrollView is used for scrollable content #1632

Merged
merged 1 commit into from
Oct 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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