Skip to content

Commit

Permalink
fix(Drawer): make spacing of header and nav robust (#1641)
Browse files Browse the repository at this point in the history 
* fix(Drawer): make spacing of header and nav robust

Ensure header and navigation is rendered outside of inner content to avoid dealing with negative margins, which again causes often a horizontal scrollbar.

* Update stories

* Update snapshots

* Update packages/dnb-eufemia/src/components/drawer/style/_drawer.scss

Co-authored-by: Anders <anderslangseth@gmail.com>

Co-authored-by: Anders <anderslangseth@gmail.com>
  • Loading branch information
@tujoworker @langz
tujoworker and langz committed Oct 17, 2022
1 parent f19d58b commit 5454ded
Show file tree
Hide file tree
Showing 15 changed files with 537 additions and 450 deletions.
56 changes: 35 additions & 21 deletions packages/dnb-eufemia/src/components/dialog/__tests__/__snapshots__/Dialog.test.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2160,32 +2160,26 @@ button.dnb-button::-moz-focus-inner {
To make the Drawer scroll, we use .dnb-scroll-view
*/
width: 100%;
height: 100%;
height: calc(100% - var(--header-height, 0));
/** 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) {
.dnb-drawer--spacing .dnb-drawer__inner {
padding: 0 calc(var(--drawer-spacing) * 1.75) 0; } }
@media screen and (max-width: 50em) {
.dnb-drawer--spacing .dnb-drawer__inner {
padding: 0 calc(var(--drawer-spacing) / 1.333333) 0; } }
@media screen and (max-width: 40em) {
.dnb-drawer--spacing .dnb-drawer__inner {
padding: 0 calc(var(--drawer-spacing) / 2) 0; } }
.dnb-drawer__align--centered .dnb-drawer__inner {
align-items: center;
justify-content: center; }
.dnb-drawer__content {
position: relative;
z-index: 1;
padding-bottom: calc(var(--drawer-spacing) * 2); }
z-index: 1; }
.dnb-drawer--spacing .dnb-drawer__content {
padding-bottom: calc(var(--drawer-spacing) * 2); }
@supports (-webkit-touch-callout: none) {
padding: 0 var(--drawer-spacing); }
@media screen and (min-width: 72em) {
.dnb-drawer--spacing .dnb-drawer__content {
padding: 0 calc(var(--drawer-spacing) * 1.75); } }
@media screen and (max-width: 50em) {
.dnb-drawer--spacing .dnb-drawer__content {
padding-bottom: calc(var(--drawer-spacing) * 8); } }
padding: 0 calc(var(--drawer-spacing) / 1.333333); } }
@media screen and (max-width: 40em) {
.dnb-drawer--spacing .dnb-drawer__content {
padding: 0 calc(var(--drawer-spacing) / 2); } }
.dnb-drawer__align--center .dnb-drawer__content {
align-items: center;
text-align: center; }
Expand Down Expand Up @@ -2253,8 +2247,19 @@ button.dnb-button::-moz-focus-inner {
.dnb-drawer .dnb-drawer__header::after {
top: -500%;
height: 600%; }
.dnb-drawer--spacing .dnb-drawer__header .dnb-tabs {
margin-top: var(--drawer-spacing); }
.dnb-drawer--spacing .dnb-drawer__header {
padding: 0 var(--drawer-spacing); }
@media screen and (min-width: 72em) {
.dnb-drawer--spacing .dnb-drawer__header {
padding: 0 calc(var(--drawer-spacing) * 1.75); } }
@media screen and (max-width: 50em) {
.dnb-drawer--spacing .dnb-drawer__header {
padding: 0 calc(var(--drawer-spacing) / 1.333333); } }
@media screen and (max-width: 40em) {
.dnb-drawer--spacing .dnb-drawer__header {
padding: 0 calc(var(--drawer-spacing) / 2); } }
.dnb-drawer--spacing .dnb-drawer__header .dnb-tabs {
margin-top: var(--drawer-spacing); }
.dnb-drawer__body {
padding-bottom: calc(var(--drawer-spacing) * 2);
margin-bottom: calc(var(--drawer-spacing-minus) * 2); }
Expand All @@ -2269,8 +2274,17 @@ button.dnb-button::-moz-focus-inner {
position: sticky;
top: 0;
z-index: 99;
margin: var(--drawer-spacing) calc(var(--drawer-spacing) * -1 * 1.75);
padding: 0 calc(var(--drawer-spacing) * 1.5); }
margin: var(--drawer-spacing) 0;
padding: 0 var(--drawer-spacing); }
@media screen and (min-width: 72em) {
.dnb-drawer--spacing .dnb-drawer__navigation.dnb-section {
padding: 0 calc(var(--drawer-spacing) * 1.75); } }
@media screen and (max-width: 50em) {
.dnb-drawer--spacing .dnb-drawer__navigation.dnb-section {
padding: 0 calc(var(--drawer-spacing) / 1.333333); } }
@media screen and (max-width: 40em) {
.dnb-drawer--spacing .dnb-drawer__navigation.dnb-section {
padding: 0 calc(var(--drawer-spacing) / 2); } }
.dnb-drawer--spacing .dnb-drawer__navigation.dnb-section.dnb-drawer__navigation--sticky {
z-index: 2999; }
.dnb-drawer .dnb-drawer__navigation--sticky::before {
Expand Down
51 changes: 39 additions & 12 deletions packages/dnb-eufemia/src/components/drawer/DrawerContent.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
isTrue,
findElementInChildren,
} from '../../shared/component-helper'
import { getOffsetTop, warn } from '../../shared/helpers'
import ScrollView from '../../fragments/scroll-view/ScrollView'
import DrawerHeader from './parts/DrawerHeader'
import DrawerNavigation from './parts/DrawerNavigation'
Expand All @@ -18,6 +19,7 @@ import { DrawerContentProps } from './types'
import { checkMinMaxWidth } from './helpers'
import ModalHeaderBar from '../modal/parts/ModalHeaderBar'
import ModalHeader from '../modal/parts/ModalHeader'
import { DrawerContentContext } from './parts/DrawerContentContext'

export default function DrawerContent({
modalContent = null,
Expand Down Expand Up @@ -69,34 +71,59 @@ export default function DrawerContent({
...rest,
}

const navExists = findElementInChildren(
/**
* Update CSS --header-height with spacing to top of page
*/
React.useEffect(() => {
try {
const height = getOffsetTop(context?.contentRef.current) / 16
context?.contentRef.current.style.setProperty(
'--header-height',
`${height}rem`
)
} catch (e) {
warn(e)
}
}, [content, context?.scrollRef, context?.contentRef])

const navigationElement = findElementInChildren(
content,
(cur) => cur.type === DrawerNavigation || cur.type === ModalHeaderBar
(cur: React.ReactElement) =>
cur.type === DrawerNavigation || cur.type === ModalHeaderBar
)

const headerExists = findElementInChildren(
const headerElement = findElementInChildren(
content,
(cur) => cur.type === DrawerHeader || cur.type === ModalHeader
(cur: React.ReactElement) =>
cur.type === DrawerHeader || cur.type === ModalHeader
)

return (
<ScrollView {...innerParams} ref={context?.scrollRef}>
{navigationElement ? (
navigationElement
) : (
<DrawerNavigation>{navContent}</DrawerNavigation>
)}

{headerElement ? (
headerElement
) : (
<DrawerHeader title={context?.title}>{headerContent}</DrawerHeader>
)}
<div
tabIndex={-1}
className="dnb-drawer__inner dnb-no-focus"
ref={context?.contentRef}
>
{!navExists && <DrawerNavigation>{navContent}</DrawerNavigation>}
{!headerExists && (
<DrawerHeader title={context?.title}>
{headerContent}
</DrawerHeader>
)}
<div
id={context?.contentId + '-content'}
className="dnb-drawer__content"
>
{content}
<DrawerContentContext.Provider
value={{ navigationElement, headerElement }}
>
{content}
</DrawerContentContext.Provider>
</div>
</div>
</ScrollView>
Expand Down
97 changes: 52 additions & 45 deletions packages/dnb-eufemia/src/components/drawer/__tests__/Drawer.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
axeComponent,
attachToBody,
} from '../../../core/jest/jestSetup'
import { render } from '@testing-library/react'
import { render, fireEvent } from '@testing-library/react'

const props = fakeProps(require.resolve('../Drawer.tsx'), {
all: true,
Expand Down Expand Up @@ -148,44 +148,44 @@ describe('Drawer', () => {
noAnimation: true,
}

const Comp = mount(
render(
<Drawer
{...props}
id="modal-first"
onOpen={on_open.first}
onClose={on_close.first}
>
<button id="content-first">content</button>
<button id="content-first">first</button>
<Drawer
{...props}
id="modal-second"
onOpen={on_open.second}
onClose={on_close.second}
>
<button id="content-second">content</button>
<button id="content-second">second</button>
<Drawer
{...props}
id="modal-third"
onOpen={on_open.third}
onClose={on_close.third}
>
<button id="content-third">content</button>
<button id="content-third">third</button>
</Drawer>
</Drawer>
</Drawer>
)

expect(Comp.exists('#content-third')).toBe(false)
expect(document.querySelector('#content-third')).toBeFalsy()

Comp.find('button#modal-first').simulate('click')
fireEvent.click(document.querySelector('button#modal-first'))
expect(
document.documentElement.getAttribute('data-dnb-modal-active')
).toBe('modal-first')
Comp.find('button#modal-second').simulate('click')
fireEvent.click(document.querySelector('button#modal-second'))
expect(
document.documentElement.getAttribute('data-dnb-modal-active')
).toBe('modal-second')
Comp.find('button#modal-third').simulate('click')
fireEvent.click(document.querySelector('button#modal-third'))
expect(
document.documentElement.getAttribute('data-dnb-modal-active')
).toBe('modal-third')
Expand All @@ -194,91 +194,90 @@ describe('Drawer', () => {
expect(on_open.second).toHaveBeenCalledTimes(1)
expect(on_open.third).toHaveBeenCalledTimes(1)

expect(Comp.find('button.dnb-modal__close-button').length).toBe(3)
expect(
Comp.find('#content-first').instance().hasAttribute('aria-hidden')
document.querySelectorAll('button.dnb-modal__close-button').length
).toBe(3)
expect(
document.querySelector('#content-first').hasAttribute('aria-hidden')
).toBe(true)
expect(
Comp.find('#content-second').instance().hasAttribute('aria-hidden')
document.querySelector('#content-second').hasAttribute('aria-hidden')
).toBe(true)
expect(
Comp.find('#content-third').instance().hasAttribute('aria-hidden')
document.querySelector('#content-third').hasAttribute('aria-hidden')
).toBe(false)

expect(
document
.querySelectorAll('button.dnb-modal__close-button')[0]
.hasAttribute('aria-hidden')
).toBe(true)
expect(
Comp.find('button.dnb-modal__close-button')
.at(0)
.instance()
document
.querySelectorAll('button.dnb-modal__close-button')[0]
.hasAttribute('aria-hidden')
).toBe(true)
expect(
Comp.find('button.dnb-modal__close-button')
.at(1)
.instance()
document
.querySelectorAll('button.dnb-modal__close-button')[1]
.hasAttribute('aria-hidden')
).toBe(true)
expect(
Comp.find('button.dnb-modal__close-button')
.at(2)
.instance()
document
.querySelectorAll('button.dnb-modal__close-button')[2]
.hasAttribute('aria-hidden')
).toBe(false)

// Close the third one
document.dispatchEvent(new KeyboardEvent('keydown', { keyCode: 27 }))
Comp.update()
expect(on_close.first).toHaveBeenCalledTimes(0)
expect(on_close.second).toHaveBeenCalledTimes(0)
expect(on_close.third).toHaveBeenCalledTimes(1)

expect(
document.documentElement.getAttribute('data-dnb-modal-active')
).toBe('modal-second')
expect(Comp.exists('#content-third')).toBe(false)
expect(document.querySelector('#content-third')).toBeFalsy()
expect(
Comp.find('#content-second').instance().hasAttribute('aria-hidden')
document.querySelector('#content-second').hasAttribute('aria-hidden')
).toBe(false)
expect(
Comp.find('button.dnb-modal__close-button')
.at(0)
.instance()
document
.querySelectorAll('button.dnb-modal__close-button')[0]
.hasAttribute('aria-hidden')
).toBe(true)
expect(
Comp.find('button.dnb-modal__close-button')
.at(1)
.instance()
document
.querySelectorAll('button.dnb-modal__close-button')[1]
.hasAttribute('aria-hidden')
).toBe(false)

// Close the second one
document.dispatchEvent(new KeyboardEvent('keydown', { keyCode: 27 }))
Comp.update()
expect(on_close.first).toHaveBeenCalledTimes(0)
expect(on_close.second).toHaveBeenCalledTimes(1)
expect(on_close.third).toHaveBeenCalledTimes(1)

expect(
document.documentElement.getAttribute('data-dnb-modal-active')
).toBe('modal-first')
expect(Comp.exists('#content-second')).toBe(false)
expect(document.querySelector('#content-second')).toBeFalsy()
expect(
Comp.find('#content-first').instance().hasAttribute('aria-hidden')
document.querySelector('#content-first').hasAttribute('aria-hidden')
).toBe(false)
expect(
Comp.find('button.dnb-modal__close-button')
.at(0)
.instance()
document
.querySelectorAll('button.dnb-modal__close-button')[0]
.hasAttribute('aria-hidden')
).toBe(false)

// Close the first one
document.dispatchEvent(new KeyboardEvent('keydown', { keyCode: 27 }))
Comp.update()
expect(on_close.first).toHaveBeenCalledTimes(1)
expect(on_close.second).toHaveBeenCalledTimes(1)
expect(on_close.third).toHaveBeenCalledTimes(1)

expect(Comp.exists('#content-first')).toBe(false)
expect(document.querySelector('#content-first')).toBeFalsy()
expect(
document.documentElement.hasAttribute('data-dnb-modal-active')
).toBe(false)
Expand Down Expand Up @@ -319,12 +318,20 @@ describe('Drawer', () => {

Comp.find('button').simulate('click')

const elements = document.querySelectorAll(
'.dnb-drawer__content > .dnb-section'
)
expect(elements[0].textContent).toContain('navigation')
expect(elements[1].textContent).toContain('header')
expect(elements[2].textContent).toContain('body')
{
const elements = document.querySelectorAll(
'.dnb-drawer.dnb-scroll-view > .dnb-section'
)
expect(elements[0].textContent).toContain('navigation')
expect(elements[1].textContent).toContain('header')
}

{
const elements = document.querySelectorAll(
'.dnb-drawer__content > .dnb-section'
)
expect(elements[0].textContent).toContain('body')
}

expect(Comp.find('button.dnb-modal__close-button').length).toBe(1)
})
Expand Down

0 comments on commit 5454ded

Please sign in to comment.