From c7c68fe33827e095ebea798f0e0243d82fb26129 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Wed, 24 Aug 2022 17:54:34 +1000 Subject: [PATCH 1/8] rename stickyTop->offsetTop and improve docs --- docs/content/PageLayout.mdx | 10 ++++++---- docs/content/SplitPageLayout.mdx | 6 +++--- src/PageLayout/PageLayout.stories.tsx | 6 +++--- src/PageLayout/PageLayout.tsx | 14 +++++++------- src/PageLayout/useStickyPaneHeight.ts | 14 +++++++------- 5 files changed, 26 insertions(+), 24 deletions(-) diff --git a/docs/content/PageLayout.mdx b/docs/content/PageLayout.mdx index 58b5195c262..75f8241df47 100644 --- a/docs/content/PageLayout.mdx +++ b/docs/content/PageLayout.mdx @@ -170,7 +170,9 @@ See [storybook](https://primer.style/react/storybook?path=/story/layout-pagelayo ### With a custom sticky header -```jsx live +Add `offsetTop` prop to specify the height of the custom sticky header along with `sticky` prop + +```jsx - + @@ -370,10 +372,10 @@ See [storybook](https://primer.style/react/storybook?path=/story/layout-pagelayo description="Whether the pane should stick to the top of the screen while the content scrolls." /> - + @@ -383,10 +383,10 @@ If you need a more flexible layout component, consider using the [PageLayout](/P description="Whether the pane should stick to the top of the screen while the content scrolls." /> ( sx={{ position: 'sticky', top: 0, - height: args.stickyTop, + height: args.offsetTop, display: 'grid', placeItems: 'center', backgroundColor: 'canvas.subtle', @@ -619,7 +619,7 @@ export const CustomStickyHeader: Story = args => ( ))} - + {Array.from({length: args.numParagraphsInPane}).map((_, i) => ( @@ -643,7 +643,7 @@ CustomStickyHeader.argTypes = { type: 'boolean', defaultValue: true }, - stickyTop: { + offsetTop: { type: 'string', defaultValue: '8rem' }, diff --git a/src/PageLayout/PageLayout.tsx b/src/PageLayout/PageLayout.tsx index b3183a060f9..614a6d03dd1 100644 --- a/src/PageLayout/PageLayout.tsx +++ b/src/PageLayout/PageLayout.tsx @@ -361,7 +361,7 @@ export type PageLayoutPaneProps = { */ dividerWhenNarrow?: 'inherit' | 'none' | 'line' | 'filled' sticky?: boolean - stickyTop?: string | number + offsetTop?: string | number hidden?: boolean | ResponsiveValue } & SxProp @@ -384,7 +384,7 @@ const Pane: React.FC> = ({ divider: responsiveDivider = 'none', dividerWhenNarrow = 'inherit', sticky = false, - stickyTop = 0, + offsetTop = 0, hidden: responsiveHidden = false, children, sx = {} @@ -411,11 +411,11 @@ const Pane: React.FC> = ({ React.useEffect(() => { if (sticky) { - enableStickyPane?.(stickyTop) + enableStickyPane?.(offsetTop) } else { disableStickyPane?.() } - }, [sticky, enableStickyPane, disableStickyPane, stickyTop]) + }, [sticky, enableStickyPane, disableStickyPane, offsetTop]) return ( > = ({ ...(sticky ? { position: 'sticky', - // If stickyTop has value, it will stick the pane to the position where the sticky top ends - // else top will be 0 as the default value of stickyTop - top: typeof stickyTop === 'number' ? `${stickyTop}px` : stickyTop, + // If offsetTop has value, it will stick the pane to the position where the sticky top ends + // else top will be 0 as the default value of offsetTop + top: typeof offsetTop === 'number' ? `${offsetTop}px` : offsetTop, overflow: 'hidden', maxHeight: 'var(--sticky-pane-height)' } diff --git a/src/PageLayout/useStickyPaneHeight.ts b/src/PageLayout/useStickyPaneHeight.ts index fa9e9ff11f0..864ab6e5790 100644 --- a/src/PageLayout/useStickyPaneHeight.ts +++ b/src/PageLayout/useStickyPaneHeight.ts @@ -10,7 +10,7 @@ export function useStickyPaneHeight() { // Default the height to the viewport height const [height, setHeight] = React.useState('100vh') - const [stickyTop, setStickyTop] = React.useState(0) + const [offsetTop, setoffsetTop] = React.useState(0) // Create intersection observers to track the top and bottom of the content region const [contentTopRef, contentTopInView, contentTopEntry] = useInView() @@ -28,8 +28,8 @@ export function useStickyPaneHeight() { const topRect = contentTopEntry?.target.getBoundingClientRect() const bottomRect = contentBottomEntry?.target.getBoundingClientRect() - - const stickyTopWithUnits = typeof stickyTop === 'number' ? `${stickyTop}px` : stickyTop + // custom sticky header's height with units + const offsetTopWithUnits = typeof offsetTop === 'number' ? `${offsetTop}px` : offsetTop if (scrollContainer) { const scrollRect = scrollContainer.getBoundingClientRect() @@ -37,7 +37,7 @@ export function useStickyPaneHeight() { const topOffset = topRect ? Math.max(topRect.top - scrollRect.top, 0) : 0 const bottomOffset = bottomRect ? Math.max(scrollRect.bottom - bottomRect.bottom, 0) : 0 - calculatedHeight = `calc(${scrollRect.height}px - (max(${topOffset}px, ${stickyTopWithUnits}) + ${bottomOffset}px))` + calculatedHeight = `calc(${scrollRect.height}px - (max(${topOffset}px, ${offsetTopWithUnits}) + ${bottomOffset}px))` } else { const topOffset = topRect ? Math.max(topRect.top, 0) : 0 const bottomOffset = bottomRect ? Math.max(window.innerHeight - bottomRect.bottom, 0) : 0 @@ -46,11 +46,11 @@ export function useStickyPaneHeight() { // We need to account for this when calculating the offset. const overflowScroll = Math.max(window.scrollY + window.innerHeight - document.body.scrollHeight, 0) - calculatedHeight = `calc(100vh - (max(${topOffset}px, ${stickyTopWithUnits}) + ${bottomOffset}px - ${overflowScroll}px))` + calculatedHeight = `calc(100vh - (max(${topOffset}px, ${offsetTopWithUnits}) + ${bottomOffset}px - ${overflowScroll}px))` } setHeight(calculatedHeight) - }, [contentTopEntry, contentBottomEntry, stickyTop]) + }, [contentTopEntry, contentBottomEntry, offsetTop]) // We only want to add scroll and resize listeners if the pane is sticky. // Since hooks can't be called conditionally, we need to use state to track @@ -92,7 +92,7 @@ export function useStickyPaneHeight() { function enableStickyPane(top: string | number) { setIsEnabled(true) - setStickyTop(top) + setoffsetTop(top) } function disableStickyPane() { From c6b8858576c83f44bfbb33396ae4bb8f858c688b Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Wed, 24 Aug 2022 18:16:44 +1000 Subject: [PATCH 2/8] add live to jsx --- docs/content/PageLayout.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/PageLayout.mdx b/docs/content/PageLayout.mdx index 75f8241df47..ad94dbd6fc6 100644 --- a/docs/content/PageLayout.mdx +++ b/docs/content/PageLayout.mdx @@ -172,7 +172,7 @@ See [storybook](https://primer.style/react/storybook?path=/story/layout-pagelayo Add `offsetTop` prop to specify the height of the custom sticky header along with `sticky` prop -```jsx +```jsx live Date: Fri, 26 Aug 2022 11:29:15 +1000 Subject: [PATCH 3/8] add changeset --- .changeset/rich-singers-double.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/rich-singers-double.md diff --git a/.changeset/rich-singers-double.md b/.changeset/rich-singers-double.md new file mode 100644 index 00000000000..dae0701a318 --- /dev/null +++ b/.changeset/rich-singers-double.md @@ -0,0 +1,5 @@ +--- +'@primer/react': major +--- + +Rename `stickyTop` prop name for the PageLayout to `offsetTop` and improve docs From 83d65f7da86c32d1906c526777f4e0005796074f Mon Sep 17 00:00:00 2001 From: Cole Bemis Date: Fri, 26 Aug 2022 12:57:48 -0700 Subject: [PATCH 4/8] Some formatting --- src/PageLayout/useStickyPaneHeight.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/PageLayout/useStickyPaneHeight.ts b/src/PageLayout/useStickyPaneHeight.ts index 864ab6e5790..9e4b603fdb8 100644 --- a/src/PageLayout/useStickyPaneHeight.ts +++ b/src/PageLayout/useStickyPaneHeight.ts @@ -10,7 +10,7 @@ export function useStickyPaneHeight() { // Default the height to the viewport height const [height, setHeight] = React.useState('100vh') - const [offsetTop, setoffsetTop] = React.useState(0) + const [offsetTop, setOffsetTop] = React.useState(0) // Create intersection observers to track the top and bottom of the content region const [contentTopRef, contentTopInView, contentTopEntry] = useInView() @@ -28,7 +28,8 @@ export function useStickyPaneHeight() { const topRect = contentTopEntry?.target.getBoundingClientRect() const bottomRect = contentBottomEntry?.target.getBoundingClientRect() - // custom sticky header's height with units + + // Custom sticky header's height with units const offsetTopWithUnits = typeof offsetTop === 'number' ? `${offsetTop}px` : offsetTop if (scrollContainer) { @@ -92,7 +93,7 @@ export function useStickyPaneHeight() { function enableStickyPane(top: string | number) { setIsEnabled(true) - setoffsetTop(top) + setOffsetTop(top) } function disableStickyPane() { From f8b275672faea5a3ef4914396b1e844705376ef9 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 29 Aug 2022 17:52:50 +1000 Subject: [PATCH 5/8] stickyTop prop name update goes as minor as no usage --- .changeset/rich-singers-double.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/rich-singers-double.md b/.changeset/rich-singers-double.md index dae0701a318..1ae5b7da713 100644 --- a/.changeset/rich-singers-double.md +++ b/.changeset/rich-singers-double.md @@ -1,5 +1,5 @@ --- -'@primer/react': major +'@primer/react': minor --- Rename `stickyTop` prop name for the PageLayout to `offsetTop` and improve docs From e463641ebcf2c6b9571bea2efdfdc2a300b6e0c2 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 29 Aug 2022 18:12:12 +1000 Subject: [PATCH 6/8] rename offsetTop->offsetHeader --- .changeset/rich-singers-double.md | 2 +- docs/content/PageLayout.mdx | 8 ++++---- docs/content/SplitPageLayout.mdx | 6 +++--- src/PageLayout/PageLayout.stories.tsx | 6 +++--- src/PageLayout/PageLayout.tsx | 14 +++++++------- src/PageLayout/useStickyPaneHeight.ts | 14 +++++++------- 6 files changed, 25 insertions(+), 25 deletions(-) diff --git a/.changeset/rich-singers-double.md b/.changeset/rich-singers-double.md index 1ae5b7da713..1a1810ad837 100644 --- a/.changeset/rich-singers-double.md +++ b/.changeset/rich-singers-double.md @@ -2,4 +2,4 @@ '@primer/react': minor --- -Rename `stickyTop` prop name for the PageLayout to `offsetTop` and improve docs +Rename `stickyTop` prop name for the PageLayout to `offsetHeader` and improve docs diff --git a/docs/content/PageLayout.mdx b/docs/content/PageLayout.mdx index ddc35cbd16e..edf26d02221 100644 --- a/docs/content/PageLayout.mdx +++ b/docs/content/PageLayout.mdx @@ -170,7 +170,7 @@ See [storybook](https://primer.style/react/storybook?path=/story/layout-pagelayo ### With a custom sticky header -Add `offsetTop` prop to specify the height of the custom sticky header along with `sticky` prop +Add `offsetHeader` prop to specify the height of the custom sticky header along with `sticky` prop ```jsx live @@ -192,7 +192,7 @@ Add `offsetTop` prop to specify the height of the custom sticky header along wit - + @@ -372,10 +372,10 @@ Add `offsetTop` prop to specify the height of the custom sticky header along wit description="Whether the pane should stick to the top of the screen while the content scrolls." /> - + @@ -383,10 +383,10 @@ If you need a more flexible layout component, consider using the [PageLayout](/P description="Whether the pane should stick to the top of the screen while the content scrolls." /> ( sx={{ position: 'sticky', top: 0, - height: args.offsetTop, + height: args.offsetHeader, display: 'grid', placeItems: 'center', backgroundColor: 'canvas.subtle', @@ -619,7 +619,7 @@ export const CustomStickyHeader: Story = args => ( ))} - + {Array.from({length: args.numParagraphsInPane}).map((_, i) => ( @@ -643,7 +643,7 @@ CustomStickyHeader.argTypes = { type: 'boolean', defaultValue: true }, - offsetTop: { + offsetHeader: { type: 'string', defaultValue: '8rem' }, diff --git a/src/PageLayout/PageLayout.tsx b/src/PageLayout/PageLayout.tsx index 614a6d03dd1..ab43e4728d3 100644 --- a/src/PageLayout/PageLayout.tsx +++ b/src/PageLayout/PageLayout.tsx @@ -361,7 +361,7 @@ export type PageLayoutPaneProps = { */ dividerWhenNarrow?: 'inherit' | 'none' | 'line' | 'filled' sticky?: boolean - offsetTop?: string | number + offsetHeader?: string | number hidden?: boolean | ResponsiveValue } & SxProp @@ -384,7 +384,7 @@ const Pane: React.FC> = ({ divider: responsiveDivider = 'none', dividerWhenNarrow = 'inherit', sticky = false, - offsetTop = 0, + offsetHeader = 0, hidden: responsiveHidden = false, children, sx = {} @@ -411,11 +411,11 @@ const Pane: React.FC> = ({ React.useEffect(() => { if (sticky) { - enableStickyPane?.(offsetTop) + enableStickyPane?.(offsetHeader) } else { disableStickyPane?.() } - }, [sticky, enableStickyPane, disableStickyPane, offsetTop]) + }, [sticky, enableStickyPane, disableStickyPane, offsetHeader]) return ( > = ({ ...(sticky ? { position: 'sticky', - // If offsetTop has value, it will stick the pane to the position where the sticky top ends - // else top will be 0 as the default value of offsetTop - top: typeof offsetTop === 'number' ? `${offsetTop}px` : offsetTop, + // If offsetHeader has value, it will stick the pane to the position where the sticky top ends + // else top will be 0 as the default value of offsetHeader + top: typeof offsetHeader === 'number' ? `${offsetHeader}px` : offsetHeader, overflow: 'hidden', maxHeight: 'var(--sticky-pane-height)' } diff --git a/src/PageLayout/useStickyPaneHeight.ts b/src/PageLayout/useStickyPaneHeight.ts index 9e4b603fdb8..5cbbdd78480 100644 --- a/src/PageLayout/useStickyPaneHeight.ts +++ b/src/PageLayout/useStickyPaneHeight.ts @@ -10,7 +10,7 @@ export function useStickyPaneHeight() { // Default the height to the viewport height const [height, setHeight] = React.useState('100vh') - const [offsetTop, setOffsetTop] = React.useState(0) + const [offsetHeader, setOffsetHeader] = React.useState(0) // Create intersection observers to track the top and bottom of the content region const [contentTopRef, contentTopInView, contentTopEntry] = useInView() @@ -28,9 +28,9 @@ export function useStickyPaneHeight() { const topRect = contentTopEntry?.target.getBoundingClientRect() const bottomRect = contentBottomEntry?.target.getBoundingClientRect() - + // Custom sticky header's height with units - const offsetTopWithUnits = typeof offsetTop === 'number' ? `${offsetTop}px` : offsetTop + const offsetHeaderWithUnits = typeof offsetHeader === 'number' ? `${offsetHeader}px` : offsetHeader if (scrollContainer) { const scrollRect = scrollContainer.getBoundingClientRect() @@ -38,7 +38,7 @@ export function useStickyPaneHeight() { const topOffset = topRect ? Math.max(topRect.top - scrollRect.top, 0) : 0 const bottomOffset = bottomRect ? Math.max(scrollRect.bottom - bottomRect.bottom, 0) : 0 - calculatedHeight = `calc(${scrollRect.height}px - (max(${topOffset}px, ${offsetTopWithUnits}) + ${bottomOffset}px))` + calculatedHeight = `calc(${scrollRect.height}px - (max(${topOffset}px, ${offsetHeaderWithUnits}) + ${bottomOffset}px))` } else { const topOffset = topRect ? Math.max(topRect.top, 0) : 0 const bottomOffset = bottomRect ? Math.max(window.innerHeight - bottomRect.bottom, 0) : 0 @@ -47,11 +47,11 @@ export function useStickyPaneHeight() { // We need to account for this when calculating the offset. const overflowScroll = Math.max(window.scrollY + window.innerHeight - document.body.scrollHeight, 0) - calculatedHeight = `calc(100vh - (max(${topOffset}px, ${offsetTopWithUnits}) + ${bottomOffset}px - ${overflowScroll}px))` + calculatedHeight = `calc(100vh - (max(${topOffset}px, ${offsetHeaderWithUnits}) + ${bottomOffset}px - ${overflowScroll}px))` } setHeight(calculatedHeight) - }, [contentTopEntry, contentBottomEntry, offsetTop]) + }, [contentTopEntry, contentBottomEntry, offsetHeader]) // We only want to add scroll and resize listeners if the pane is sticky. // Since hooks can't be called conditionally, we need to use state to track @@ -93,7 +93,7 @@ export function useStickyPaneHeight() { function enableStickyPane(top: string | number) { setIsEnabled(true) - setOffsetTop(top) + setOffsetHeader(top) } function disableStickyPane() { From 5b52dc7d603f76ca2a9b18a021deabd13168c1e4 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 5 Sep 2022 18:57:21 +1000 Subject: [PATCH 7/8] update stickyTop arg name to offsetHeader --- src/PageLayout/interaction.stories.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PageLayout/interaction.stories.tsx b/src/PageLayout/interaction.stories.tsx index 6c225176b0d..f60dce07369 100644 --- a/src/PageLayout/interaction.stories.tsx +++ b/src/PageLayout/interaction.stories.tsx @@ -395,7 +395,7 @@ CustomStickyHeader.argTypes = { type: 'boolean', defaultValue: true }, - stickyTop: { + offsetHeader: { type: 'string', defaultValue: '8rem' }, From 4564242f719f524fb508ee0885553d9f7407f330 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Mon, 5 Sep 2022 20:18:09 +1000 Subject: [PATCH 8/8] add to content to trigger new scnapshot generate from chromatic --- src/PageLayout/PageLayout.stories.tsx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/PageLayout/PageLayout.stories.tsx b/src/PageLayout/PageLayout.stories.tsx index aad5b1903f1..874847feadc 100644 --- a/src/PageLayout/PageLayout.stories.tsx +++ b/src/PageLayout/PageLayout.stories.tsx @@ -624,13 +624,13 @@ export const CustomStickyHeader: Story = args => ( return ( - Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nam at enim id lorem tempus egestas a non - ipsum. Maecenas imperdiet ante quam, at varius lorem molestie vel. Sed at eros consequat, varius - tellus et, auctor felis. Donec pulvinar lacinia urna nec commodo. Phasellus at imperdiet risus. Donec - sit amet massa purus. Nunc sem lectus, bibendum a sapien nec, tristique tempus felis. Ut porttitor - auctor tellus in imperdiet. Ut blandit tincidunt augue, quis fringilla nunc tincidunt sed. Vestibulum - auctor euismod nisi. Nullam tincidunt est in mi tincidunt dictum. Sed consectetur aliquet velit ut - ornare. + Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin vitae orci et magna consectetur + ullamcorper eget ac purus. Nam at enim id lorem tempus egestas a non ipsum. Maecenas imperdiet ante + quam, at varius lorem molestie vel. Sed at eros consequat, varius tellus et, auctor felis. Donec + pulvinar lacinia urna nec commodo. Phasellus at imperdiet risus. Donec sit amet massa purus. Nunc sem + lectus, bibendum a sapien nec, tristique tempus felis. Ut porttitor auctor tellus in imperdiet. Ut + blandit tincidunt augue, quis fringilla nunc tincidunt sed. Vestibulum auctor euismod nisi. Nullam + tincidunt est in mi tincidunt dictum. Sed consectetur aliquet velit ut ornare. )