From 3e99f12567841deada137bea95501a14a4c2e084 Mon Sep 17 00:00:00 2001 From: Alistair Gempf Date: Mon, 9 Dec 2019 16:00:22 +0000 Subject: [PATCH 1/6] make isScrollable a prop --- .../src/ScrollableNavigation/index.jsx | 11 +- .../src/__snapshots__/index.test.jsx.snap | 312 +++++++++++++++++- .../psammead-navigation/src/index.stories.jsx | 13 +- .../psammead-navigation/src/index.test.jsx | 16 +- 4 files changed, 337 insertions(+), 15 deletions(-) diff --git a/packages/components/psammead-navigation/src/ScrollableNavigation/index.jsx b/packages/components/psammead-navigation/src/ScrollableNavigation/index.jsx index 2e6f191258..cb8e270093 100644 --- a/packages/components/psammead-navigation/src/ScrollableNavigation/index.jsx +++ b/packages/components/psammead-navigation/src/ScrollableNavigation/index.jsx @@ -1,8 +1,7 @@ import React from 'react'; import styled, { css } from 'styled-components'; -import { node, oneOf, string } from 'prop-types'; +import { node, oneOf, string, bool } from 'prop-types'; import { GEL_GROUP_2_SCREEN_WIDTH_MAX } from '@bbc/gel-foundations/breakpoints'; -import useMediaQuery from '../hooks/useMediaQuery'; /* Convert C_POSTBOX to rgba as IE doesn't like 8 digit hex */ const C_POSTBOX_TRANSPARENT = `rgba(184, 0, 0, 0)`; @@ -43,8 +42,11 @@ const StyledScrollableNav = styled.div` } `; -export const CanonicalScrollableNavigation = ({ children, dir }) => { - const isScrollable = useMediaQuery('(max-width: 600px)'); +export const CanonicalScrollableNavigation = ({ + children, + dir, + isScrollable, +}) => { const ariaHidden = isScrollable && { 'aria-hidden': true }; return ( @@ -59,6 +61,7 @@ export const CanonicalScrollableNavigation = ({ children, dir }) => { CanonicalScrollableNavigation.propTypes = { children: node.isRequired, dir: oneOf(['ltr', 'rtl']), + isScrollable: bool.isRequired, }; CanonicalScrollableNavigation.defaultProps = { dir: 'ltr' }; diff --git a/packages/components/psammead-navigation/src/__snapshots__/index.test.jsx.snap b/packages/components/psammead-navigation/src/__snapshots__/index.test.jsx.snap index 0dbee7664a..4c8ca2f896 100644 --- a/packages/components/psammead-navigation/src/__snapshots__/index.test.jsx.snap +++ b/packages/components/psammead-navigation/src/__snapshots__/index.test.jsx.snap @@ -1115,7 +1115,317 @@ exports[`Scrollable Navigation should render AMP version correctly 1`] = ` `; -exports[`Scrollable Navigation should render Canonical version correctly 1`] = ` +exports[`Scrollable Navigation should render non-scrollable Canonical version correctly 1`] = ` +.c8 { + -webkit-clip-path: inset(100%); + clip-path: inset(100%); + -webkit-clip: rect(1px,1px,1px,1px); + clip: rect(1px,1px,1px,1px); + height: 1px; + overflow: hidden; + position: absolute; + width: 1px; + margin: 0; +} + +.c2 { + position: relative; + max-width: 80rem; + margin: 0 auto; +} + +.c3 { + list-style-type: none; + padding: 0; + margin: 0; + position: relative; +} + +.c6 { + font-size: 0.9375rem; + line-height: 1.25rem; + font-family: ReithSans,Helvetica,Arial,sans-serif; + font-weight: 400; + font-style: normal; + color: #FDFDFD; + cursor: pointer; + -webkit-text-decoration: none; + text-decoration: none; + display: inline-block; + padding: 0.75rem 1rem; +} + +.c6:hover::after { + content: ''; + position: absolute; + left: 0; + right: 0; + bottom: 0; + border-bottom: 0.25rem solid #FFFFFF; + border-bottom: 0.3125rem solid #FFFFFF; +} + +.c6:focus::after { + content: ''; + position: absolute; + left: 0; + right: 0; + bottom: 0; + border-bottom: 0.25rem solid #FFFFFF; + top: 0; + border: 0.25rem solid #FFFFFF; +} + +.c9 { + font-size: 0.9375rem; + line-height: 1.25rem; + font-family: ReithSans,Helvetica,Arial,sans-serif; + font-weight: 400; + font-style: normal; + color: #FDFDFD; + cursor: pointer; + -webkit-text-decoration: none; + text-decoration: none; + display: inline-block; + padding: 0.75rem 1rem; +} + +.c9:hover::after { + content: ''; + position: absolute; + left: 0; + right: 0; + bottom: 0; + border-bottom: 0.25rem solid #FFFFFF; +} + +.c9:focus::after { + content: ''; + position: absolute; + left: 0; + right: 0; + bottom: 0; + border-bottom: 0.25rem solid #FFFFFF; + top: 0; + border: 0.25rem solid #FFFFFF; +} + +.c5 { + display: inline-block; + position: relative; + z-index: 2; +} + +.c7::after { + content: ''; + position: absolute; + left: 0; + right: 0; + bottom: 0; + border-bottom: 0.25rem solid #FFFFFF; +} + +.c1 { + position: relative; + background-color: #B80000; + border-top: 0.0625rem solid #FFFFFF; +} + +.c1 .c4::after { + left: 0; +} + +.c10 .c4::after { + left: 0; +} + +.c11 .c4::after { + left: 0; +} + +@media (max-width:37.4375rem) { + .c0 { + white-space: nowrap; + overflow-x: scroll; + -webkit-scroll-behavior: smooth; + -moz-scroll-behavior: smooth; + -ms-scroll-behavior: smooth; + scroll-behavior: smooth; + -webkit-overflow-scrolling: touch; + -webkit-scrollbar-width: none; + -moz-scrollbar-width: none; + -ms-scrollbar-width: none; + scrollbar-width: none; + -ms-overflow-style: none; + } + + .c0::-webkit-scrollbar { + display: none; + } + + .c0:after { + content: ' '; + height: 100%; + width: 3rem; + position: absolute; + right: 0; + bottom: 0; + z-index: 3; + overflow: hidden; + pointer-events: none; + background: linear-gradient( to right,rgba(184,0,0,0) 0%,rgba(184,0,0,1) 100% ); + } +} + +@media (min-width:37.5rem) { + .c3 { + overflow: hidden; + } +} + +@media (min-width:20rem) and (max-width:37.4375rem) { + .c6 { + font-size: 1rem; + line-height: 1.25rem; + } +} + +@media (min-width:37.5rem) { + .c6 { + font-size: 1rem; + line-height: 1.25rem; + } +} + +@media (max-width:24.9375rem) { + .c6 { + padding: 0.75rem 0.5rem; + } +} + +@media (min-width:20rem) and (max-width:37.4375rem) { + .c9 { + font-size: 1rem; + line-height: 1.25rem; + } +} + +@media (min-width:37.5rem) { + .c9 { + font-size: 1rem; + line-height: 1.25rem; + } +} + +@media (max-width:24.9375rem) { + .c9 { + padding: 0.75rem 0.5rem; + } +} + +@media (max-width:37.4375rem) { + .c5:last-child { + margin-right: 3rem; + } +} + +@media (min-width:37.5rem) { + .c5::after { + content: ''; + position: absolute; + bottom: 0; + width: 80rem; + border-bottom: 0.0625rem solid #eab3b3; + z-index: -1; + } +} + +
+ +
+`; + +exports[`Scrollable Navigation should render scrollable Canonical version correctly 1`] = ` .c8 { -webkit-clip-path: inset(100%); clip-path: inset(100%); diff --git a/packages/components/psammead-navigation/src/index.stories.jsx b/packages/components/psammead-navigation/src/index.stories.jsx index 523655ef10..b2d6ecf099 100644 --- a/packages/components/psammead-navigation/src/index.stories.jsx +++ b/packages/components/psammead-navigation/src/index.stories.jsx @@ -1,6 +1,7 @@ import React from 'react'; import { storiesOf } from '@storybook/react'; import styled from 'styled-components'; +import { useEffect, useState } from '@storybook/addons'; import { color, select, @@ -146,6 +147,16 @@ const navigationStory = ( ? AmpScrollableNavigation : CanonicalScrollableNavigation; + const [isScrollable, setIsScrollable] = useState(false); + + useEffect(() => { + const mediaQueryList = window.matchMedia('(max-width: 37.5rem)'); + setIsScrollable(mediaQueryList.matches); + mediaQueryList.addListener(event => { + setIsScrollable(event.matches); + }); + }, []); + return ( <> {brand && getBrand()} @@ -156,7 +167,7 @@ const navigationStory = ( service={service} dir={dir} > - + {navData.map((item, index) => { const { title, url } = item; diff --git a/packages/components/psammead-navigation/src/index.test.jsx b/packages/components/psammead-navigation/src/index.test.jsx index e70f3870b5..ae9ba7ab62 100644 --- a/packages/components/psammead-navigation/src/index.test.jsx +++ b/packages/components/psammead-navigation/src/index.test.jsx @@ -67,17 +67,15 @@ describe('Navigation', () => { }); describe('Scrollable Navigation', () => { - window.matchMedia = jest.fn().mockImplementation(query => { - return { - matches: true, - media: query, - addListener: jest.fn(), - removeListener: jest.fn(), - }; - }); + shouldMatchSnapshot( + 'should render scrollable Canonical version correctly', + + {NavigationExample} + , + ); shouldMatchSnapshot( - 'should render Canonical version correctly', + 'should render non-scrollable Canonical version correctly', {NavigationExample} , From 60806a66b272849240bba60d55f31e94cbb03bf5 Mon Sep 17 00:00:00 2001 From: Alistair Gempf Date: Mon, 9 Dec 2019 16:06:13 +0000 Subject: [PATCH 2/6] Remove media query hook --- .../psammead-navigation/package.json | 2 +- .../src/hooks/useMediaQuery.js | 21 ----------- .../src/hooks/useMediaQuery.test.js | 35 ------------------- 3 files changed, 1 insertion(+), 57 deletions(-) delete mode 100644 packages/components/psammead-navigation/src/hooks/useMediaQuery.js delete mode 100644 packages/components/psammead-navigation/src/hooks/useMediaQuery.test.js diff --git a/packages/components/psammead-navigation/package.json b/packages/components/psammead-navigation/package.json index 765610525d..95ac200d4f 100644 --- a/packages/components/psammead-navigation/package.json +++ b/packages/components/psammead-navigation/package.json @@ -1,6 +1,6 @@ { "name": "@bbc/psammead-navigation", - "version": "6.0.0-alpha.12", + "version": "6.0.0-alpha.13", "description": "A navigation bar to use on index pages", "main": "dist/index.js", "module": "esm/index.js", diff --git a/packages/components/psammead-navigation/src/hooks/useMediaQuery.js b/packages/components/psammead-navigation/src/hooks/useMediaQuery.js deleted file mode 100644 index 9820683c60..0000000000 --- a/packages/components/psammead-navigation/src/hooks/useMediaQuery.js +++ /dev/null @@ -1,21 +0,0 @@ -import { useState, useEffect } from 'react'; - -const useMediaQuery = query => { - const [matches, setMatches] = useState(null); - - useEffect(() => { - const mediaQueryList = window.matchMedia(query); - const mediaMatches = mediaQueryList.matches; - - setMatches(mediaMatches); - - const handler = event => setMatches(event.matches); - - mediaQueryList.addListener(handler); - return () => mediaQueryList.removeListener(handler); - }, []); - - return matches; -}; - -export default useMediaQuery; diff --git a/packages/components/psammead-navigation/src/hooks/useMediaQuery.test.js b/packages/components/psammead-navigation/src/hooks/useMediaQuery.test.js deleted file mode 100644 index 84bf5df99d..0000000000 --- a/packages/components/psammead-navigation/src/hooks/useMediaQuery.test.js +++ /dev/null @@ -1,35 +0,0 @@ -import { renderHook, act } from '@testing-library/react-hooks'; -import useMediaQuery from './useMediaQuery'; - -describe('MediaQuery hook', () => { - const mockAddListener = jest.fn(); - const mockRemoveListener = jest.fn(); - - window.matchMedia = jest.fn().mockImplementation(query => { - return { - matches: query === '(max-width: 600px)', - addListener: mockAddListener, - removeListener: mockRemoveListener, - }; - }); - - beforeEach(() => { - mockAddListener.mockReset(); - mockRemoveListener.mockReset(); - }); - - it('it should test that the media query matches', () => { - const { result } = renderHook(() => useMediaQuery('(max-width: 600px)')); - expect(result.current).toBe(true); - expect(mockAddListener).toBeCalled(); - }); - - it('should add a listener that updates the state', () => { - const { result } = renderHook(() => useMediaQuery('(max-width: 600px)')); - - act(() => mockAddListener.mock.calls[0][0]({ matches: false })); - expect(result.current).toBe(false); - act(() => mockAddListener.mock.calls[0][0]({ matches: true })); - expect(result.current).toBe(true); - }); -}); From 219af85b04560091cea404534a21223185d2dde4 Mon Sep 17 00:00:00 2001 From: Alistair Gempf Date: Mon, 9 Dec 2019 16:06:59 +0000 Subject: [PATCH 3/6] Update package lock --- packages/components/psammead-navigation/package-lock.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/psammead-navigation/package-lock.json b/packages/components/psammead-navigation/package-lock.json index a23222b09c..88636f1a18 100644 --- a/packages/components/psammead-navigation/package-lock.json +++ b/packages/components/psammead-navigation/package-lock.json @@ -1,6 +1,6 @@ { "name": "@bbc/psammead-navigation", - "version": "6.0.0-alpha.12", + "version": "6.0.0-alpha.13", "lockfileVersion": 1, "requires": true, "dependencies": { From ce49267ce935877c14d3aa860a4879a04cc402e2 Mon Sep 17 00:00:00 2001 From: Alistair Gempf Date: Mon, 9 Dec 2019 16:15:14 +0000 Subject: [PATCH 4/6] Update changelog --- packages/components/psammead-navigation/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/psammead-navigation/CHANGELOG.md b/packages/components/psammead-navigation/CHANGELOG.md index 7e2b93b7bf..c74b6f0fa1 100644 --- a/packages/components/psammead-navigation/CHANGELOG.md +++ b/packages/components/psammead-navigation/CHANGELOG.md @@ -3,6 +3,7 @@ | Version | Description | |---------|-------------| +| 6.0.0-alpha.13 | [PR#2801](https://github.com/bbc/psammead/pull/2801) Remove media query hooks and use props | | 6.0.0-alpha.12 | [PR#2799](https://github.com/bbc/psammead/pull/2799) Refactor Canonical MenuButton props | | 6.0.0-alpha.11 | [PR#2787](https://github.com/bbc/psammead/pull/2787) Update useMediaQuery hook to ensure server/client markup is consistent | | 6.0.0-alpha.10 | [PR#2785](https://github.com/bbc/psammead/pull/2785) Add `id`, `ampOpenClass` props to `Navigation` | From 2d344145124bc3089461e5c963a8ffcaa444b9b8 Mon Sep 17 00:00:00 2001 From: Alistair Gempf Date: Tue, 10 Dec 2019 09:36:29 +0000 Subject: [PATCH 5/6] Remove listener from storybook at end of useEffect --- .../components/psammead-navigation/src/index.stories.jsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/components/psammead-navigation/src/index.stories.jsx b/packages/components/psammead-navigation/src/index.stories.jsx index b2d6ecf099..ebe069d0f0 100644 --- a/packages/components/psammead-navigation/src/index.stories.jsx +++ b/packages/components/psammead-navigation/src/index.stories.jsx @@ -152,9 +152,11 @@ const navigationStory = ( useEffect(() => { const mediaQueryList = window.matchMedia('(max-width: 37.5rem)'); setIsScrollable(mediaQueryList.matches); - mediaQueryList.addListener(event => { - setIsScrollable(event.matches); - }); + + const handler = event => setIsScrollable(event.matches); + mediaQueryList.addListener(handler); + + return () => mediaQueryList.removeListener(handler); }, []); return ( From c8b849b4c0f86fd6f161b085fe7c478bf4b1c67e Mon Sep 17 00:00:00 2001 From: Alistair Gempf Date: Tue, 10 Dec 2019 09:37:26 +0000 Subject: [PATCH 6/6] Default isScrollable to false --- .../psammead-navigation/src/ScrollableNavigation/index.jsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/components/psammead-navigation/src/ScrollableNavigation/index.jsx b/packages/components/psammead-navigation/src/ScrollableNavigation/index.jsx index cb8e270093..4858a6ac36 100644 --- a/packages/components/psammead-navigation/src/ScrollableNavigation/index.jsx +++ b/packages/components/psammead-navigation/src/ScrollableNavigation/index.jsx @@ -61,7 +61,11 @@ export const CanonicalScrollableNavigation = ({ CanonicalScrollableNavigation.propTypes = { children: node.isRequired, dir: oneOf(['ltr', 'rtl']), - isScrollable: bool.isRequired, + isScrollable: bool, +}; + +CanonicalScrollableNavigation.defaultProps = { + isScrollable: false, }; CanonicalScrollableNavigation.defaultProps = { dir: 'ltr' };