From 82fd8c35e4017b34f03119d8eb99188b34bbd713 Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Fri, 5 Aug 2022 09:38:22 -0400 Subject: [PATCH] Change `createSlots` to use layout effects when registering slots (#2216) * Change createSlots to use layout effects instead of regular effects * Update snapshots * Create hip-buses-eat.md --- .changeset/hip-buses-eat.md | 5 + .../__snapshots__/CheckboxGroup.test.tsx.snap | 102 ++- .../CheckboxOrRadioGroup.test.tsx.snap | 102 ++- .../__snapshots__/RadioGroup.test.tsx.snap | 102 ++- .../ChoiceFieldset.test.tsx.snap | 734 +++++++++++++++++- src/utils/create-slots.tsx | 5 +- 6 files changed, 973 insertions(+), 77 deletions(-) create mode 100644 .changeset/hip-buses-eat.md diff --git a/.changeset/hip-buses-eat.md b/.changeset/hip-buses-eat.md new file mode 100644 index 00000000000..8e9e703d996 --- /dev/null +++ b/.changeset/hip-buses-eat.md @@ -0,0 +1,5 @@ +--- +"@primer/react": patch +--- + +Change `createSlots` to use layout effects when registering slots diff --git a/src/__tests__/__snapshots__/CheckboxGroup.test.tsx.snap b/src/__tests__/__snapshots__/CheckboxGroup.test.tsx.snap index 6b91387659b..8f762d1c513 100644 --- a/src/__tests__/__snapshots__/CheckboxGroup.test.tsx.snap +++ b/src/__tests__/__snapshots__/CheckboxGroup.test.tsx.snap @@ -12,19 +12,46 @@ exports[`CheckboxGroup renders consistently 1`] = ` padding: 0; } -.c3 { +.c4 { display: -webkit-box; display: -webkit-flex; display: -ms-flexbox; display: flex; } -.c4 > input { +.c5 > input { margin-left: 0; margin-right: 0; } +.c7 { + margin-left: 8px; + display: -webkit-box; + display: -webkit-flex; + display: -ms-flexbox; + display: flex; + -webkit-flex-direction: column; + -ms-flex-direction: column; + flex-direction: column; +} + .c2 { + display: block; + font-size: 16px; +} + +.c8 { + font-weight: 600; + font-size: 14px; + display: block; + color: #24292f; + cursor: pointer; + -webkit-align-self: flex-start; + -ms-flex-item-align: start; + align-self: flex-start; +} + +.c3 { display: -webkit-box; display: -webkit-flex; display: -ms-flexbox; @@ -37,25 +64,25 @@ exports[`CheckboxGroup renders consistently 1`] = ` padding: 0; } -.c2 > * + * { +.c3 > * + * { margin-top: 8px; } -.c5 { +.c6 { cursor: pointer; } -.c5:focus:not(:disabled) { +.c6:focus:not(:disabled) { box-shadow: none; outline: 2px solid #0969da; outline-offset: 0; } -.c5:focus:not(:disabled):not(:focus-visible) { +.c6:focus:not(:disabled):not(:focus-visible) { outline: solid 1px transparent; } -.c5:focus-visible:not(:disabled) { +.c6:focus-visible:not(:disabled) { box-shadow: none; outline: 2px solid #0969da; outline-offset: 0; @@ -68,23 +95,29 @@ exports[`CheckboxGroup renders consistently 1`] = ` > + > + + Choices + +
+
+ +
+
+ +
+
+ +
diff --git a/src/__tests__/__snapshots__/CheckboxOrRadioGroup.test.tsx.snap b/src/__tests__/__snapshots__/CheckboxOrRadioGroup.test.tsx.snap index c7b60faf0b8..035bafc69c7 100644 --- a/src/__tests__/__snapshots__/CheckboxOrRadioGroup.test.tsx.snap +++ b/src/__tests__/__snapshots__/CheckboxOrRadioGroup.test.tsx.snap @@ -12,19 +12,46 @@ exports[`CheckboxOrRadioGroup renders consistently 1`] = ` padding: 0; } -.c3 { +.c4 { display: -webkit-box; display: -webkit-flex; display: -ms-flexbox; display: flex; } -.c4 > input { +.c5 > input { margin-left: 0; margin-right: 0; } +.c7 { + margin-left: 8px; + display: -webkit-box; + display: -webkit-flex; + display: -ms-flexbox; + display: flex; + -webkit-flex-direction: column; + -ms-flex-direction: column; + flex-direction: column; +} + .c2 { + display: block; + font-size: 16px; +} + +.c8 { + font-weight: 600; + font-size: 14px; + display: block; + color: #24292f; + cursor: pointer; + -webkit-align-self: flex-start; + -ms-flex-item-align: start; + align-self: flex-start; +} + +.c3 { display: -webkit-box; display: -webkit-flex; display: -ms-flexbox; @@ -37,25 +64,25 @@ exports[`CheckboxOrRadioGroup renders consistently 1`] = ` padding: 0; } -.c2 > * + * { +.c3 > * + * { margin-top: 8px; } -.c5 { +.c6 { cursor: pointer; } -.c5:focus:not(:disabled) { +.c6:focus:not(:disabled) { box-shadow: none; outline: 2px solid #0969da; outline-offset: 0; } -.c5:focus:not(:disabled):not(:focus-visible) { +.c6:focus:not(:disabled):not(:focus-visible) { outline: solid 1px transparent; } -.c5:focus-visible:not(:disabled) { +.c6:focus-visible:not(:disabled) { box-shadow: none; outline: 2px solid #0969da; outline-offset: 0; @@ -68,23 +95,29 @@ exports[`CheckboxOrRadioGroup renders consistently 1`] = ` > + > + + Choices + +
+
+ +
+
+ +
+
+ +
diff --git a/src/__tests__/__snapshots__/RadioGroup.test.tsx.snap b/src/__tests__/__snapshots__/RadioGroup.test.tsx.snap index f4e59c352ff..a26bf50a45f 100644 --- a/src/__tests__/__snapshots__/RadioGroup.test.tsx.snap +++ b/src/__tests__/__snapshots__/RadioGroup.test.tsx.snap @@ -12,19 +12,46 @@ exports[`RadioGroup renders consistently 1`] = ` padding: 0; } -.c3 { +.c4 { display: -webkit-box; display: -webkit-flex; display: -ms-flexbox; display: flex; } -.c4 > input { +.c5 > input { margin-left: 0; margin-right: 0; } +.c7 { + margin-left: 8px; + display: -webkit-box; + display: -webkit-flex; + display: -ms-flexbox; + display: flex; + -webkit-flex-direction: column; + -ms-flex-direction: column; + flex-direction: column; +} + .c2 { + display: block; + font-size: 16px; +} + +.c8 { + font-weight: 600; + font-size: 14px; + display: block; + color: #24292f; + cursor: pointer; + -webkit-align-self: flex-start; + -ms-flex-item-align: start; + align-self: flex-start; +} + +.c3 { display: -webkit-box; display: -webkit-flex; display: -ms-flexbox; @@ -37,25 +64,25 @@ exports[`RadioGroup renders consistently 1`] = ` padding: 0; } -.c2 > * + * { +.c3 > * + * { margin-top: 8px; } -.c5 { +.c6 { cursor: pointer; } -.c5:focus:not(:disabled) { +.c6:focus:not(:disabled) { box-shadow: none; outline: 2px solid #0969da; outline-offset: 0; } -.c5:focus:not(:disabled):not(:focus-visible) { +.c6:focus:not(:disabled):not(:focus-visible) { outline: solid 1px transparent; } -.c5:focus-visible:not(:disabled) { +.c6:focus-visible:not(:disabled) { box-shadow: none; outline: 2px solid #0969da; outline-offset: 0; @@ -68,23 +95,29 @@ exports[`RadioGroup renders consistently 1`] = ` > + > + + Choices + +
+
+ +
+
+ +
+
+ +
diff --git a/src/__tests__/deprecated/__snapshots__/ChoiceFieldset.test.tsx.snap b/src/__tests__/deprecated/__snapshots__/ChoiceFieldset.test.tsx.snap index 8acc1d06fad..ffd119d38a6 100644 --- a/src/__tests__/deprecated/__snapshots__/ChoiceFieldset.test.tsx.snap +++ b/src/__tests__/deprecated/__snapshots__/ChoiceFieldset.test.tsx.snap @@ -651,14 +651,209 @@ exports[`ChoiceFieldset renders a list with selected items 1`] = ` border: none; } +.c1 { + margin-bottom: 16px; +} + +.c4 { + display: -webkit-box; + display: -webkit-flex; + display: -ms-flexbox; + display: flex; +} + +.c5 > input { + margin-left: 0; + margin-right: 0; +} + +.c7 { + margin-left: 8px; + display: -webkit-box; + display: -webkit-flex; + display: -ms-flexbox; + display: flex; + -webkit-flex-direction: column; + -ms-flex-direction: column; + flex-direction: column; +} + +.c2 { + font-size: 16px; + padding: 0; +} + +.c8 { + font-weight: 600; + font-size: 14px; + display: block; + color: #24292f; + cursor: pointer; + -webkit-align-self: flex-start; + -ms-flex-item-align: start; + align-self: flex-start; +} + +.c6 { + cursor: pointer; +} + +.c6:focus:not(:disabled) { + box-shadow: none; + outline: 2px solid #0969da; + outline-offset: 0; +} + +.c6:focus:not(:disabled):not(:focus-visible) { + outline: solid 1px transparent; +} + +.c6:focus-visible:not(:disabled) { + box-shadow: none; + outline: 2px solid #0969da; + outline-offset: 0; +} + +.c3 { + display: -webkit-box; + display: -webkit-flex; + display: -ms-flexbox; + display: flex; + -webkit-flex-direction: column; + -ms-flex-direction: column; + flex-direction: column; + list-style: none; + margin: 0; + padding: 0; +} + +.c3 > li + li { + margin-top: 8px; +} +
+ className="c1" + > + + Legend + +
+
    +
  • +
    +
    + +
    +
    + +
    +
    +
  • +
  • +
    +
    + +
    +
    + +
    +
    +
  • +
  • +
    +
    + +
    +
    + +
    +
    +
  • +
`; @@ -670,42 +865,459 @@ exports[`ChoiceFieldset renders a required fieldset 1`] = ` border: none; } +.c1 { + margin-bottom: 16px; +} + +.c3 { + display: -webkit-box; + display: -webkit-flex; + display: -ms-flexbox; + display: flex; +} + +.c6 > input { + margin-left: 0; + margin-right: 0; +} + +.c8 { + margin-left: 8px; + display: -webkit-box; + display: -webkit-flex; + display: -ms-flexbox; + display: flex; + -webkit-flex-direction: column; + -ms-flex-direction: column; + flex-direction: column; +} + +.c4 { + margin-right: 4px; +} + +.c2 { + font-size: 16px; + padding: 0; +} + +.c9 { + font-weight: 600; + font-size: 14px; + display: block; + color: #24292f; + cursor: pointer; + -webkit-align-self: flex-start; + -ms-flex-item-align: start; + align-self: flex-start; +} + +.c7 { + cursor: pointer; +} + +.c7:focus:not(:disabled) { + box-shadow: none; + outline: 2px solid #0969da; + outline-offset: 0; +} + +.c7:focus:not(:disabled):not(:focus-visible) { + outline: solid 1px transparent; +} + +.c7:focus-visible:not(:disabled) { + box-shadow: none; + outline: 2px solid #0969da; + outline-offset: 0; +} + +.c5 { + display: -webkit-box; + display: -webkit-flex; + display: -ms-flexbox; + display: flex; + -webkit-flex-direction: column; + -ms-flex-direction: column; + flex-direction: column; + list-style: none; + margin: 0; + padding: 0; +} + +.c5 > li + li { + margin-top: 8px; +} + +
+
+
+ + +
+ Legend +
+ + * + +
+
+
+
    +
  • +
    +
    + +
    +
    + +
    +
    +
  • +
  • +
    +
    + +
    +
    + +
    +
    +
  • +
+
+
+`; + +exports[`ChoiceFieldset renders default 1`] = ` +.c0 { + margin: 0; + padding: 0; + border: none; +} + +.c1 { + margin-bottom: 16px; +} + +.c4 { + display: -webkit-box; + display: -webkit-flex; + display: -ms-flexbox; + display: flex; +} + +.c5 > input { + margin-left: 0; + margin-right: 0; +} + +.c7 { + margin-left: 8px; + display: -webkit-box; + display: -webkit-flex; + display: -ms-flexbox; + display: flex; + -webkit-flex-direction: column; + -ms-flex-direction: column; + flex-direction: column; +} + +.c2 { + font-size: 16px; + padding: 0; +} + +.c8 { + font-weight: 600; + font-size: 14px; + display: block; + color: #24292f; + cursor: pointer; + -webkit-align-self: flex-start; + -ms-flex-item-align: start; + align-self: flex-start; +} + +.c6 { + cursor: pointer; +} + +.c6:focus:not(:disabled) { + box-shadow: none; + outline: 2px solid #0969da; + outline-offset: 0; +} + +.c6:focus:not(:disabled):not(:focus-visible) { + outline: solid 1px transparent; +} + +.c6:focus-visible:not(:disabled) { + box-shadow: none; + outline: 2px solid #0969da; + outline-offset: 0; +} + +.c3 { + display: -webkit-box; + display: -webkit-flex; + display: -ms-flexbox; + display: flex; + -webkit-flex-direction: column; + -ms-flex-direction: column; + flex-direction: column; + list-style: none; + margin: 0; + padding: 0; +} + +.c3 > li + li { + margin-top: 8px; +} +
+ className="c1" + > + + Legend + +
+
    +
  • +
    +
    + +
    +
    + +
    +
    +
  • +
  • +
    +
    + +
    +
    + +
    +
    +
  • +
`; -exports[`ChoiceFieldset renders default 1`] = ` +exports[`ChoiceFieldset renders with a custom name and id passed 1`] = ` .c0 { margin: 0; padding: 0; border: none; } -
-
-
-
-
-`; +.c1 { + margin-bottom: 16px; +} -exports[`ChoiceFieldset renders with a custom name and id passed 1`] = ` -.c0 { +.c4 { + display: -webkit-box; + display: -webkit-flex; + display: -ms-flexbox; + display: flex; +} + +.c5 > input { + margin-left: 0; + margin-right: 0; +} + +.c7 { + margin-left: 8px; + display: -webkit-box; + display: -webkit-flex; + display: -ms-flexbox; + display: flex; + -webkit-flex-direction: column; + -ms-flex-direction: column; + flex-direction: column; +} + +.c2 { + font-size: 16px; + padding: 0; +} + +.c8 { + font-weight: 600; + font-size: 14px; + display: block; + color: #24292f; + cursor: pointer; + -webkit-align-self: flex-start; + -ms-flex-item-align: start; + align-self: flex-start; +} + +.c6 { + cursor: pointer; +} + +.c6:focus:not(:disabled) { + box-shadow: none; + outline: 2px solid #0969da; + outline-offset: 0; +} + +.c6:focus:not(:disabled):not(:focus-visible) { + outline: solid 1px transparent; +} + +.c6:focus-visible:not(:disabled) { + box-shadow: none; + outline: 2px solid #0969da; + outline-offset: 0; +} + +.c3 { + display: -webkit-box; + display: -webkit-flex; + display: -ms-flexbox; + display: flex; + -webkit-flex-direction: column; + -ms-flex-direction: column; + flex-direction: column; + list-style: none; margin: 0; padding: 0; - border: none; +} + +.c3 > li + li { + margin-top: 8px; }
@@ -714,8 +1326,88 @@ exports[`ChoiceFieldset renders with a custom name and id passed 1`] = ` className="c0" >
+ className="c1" + > + + Legend + +
+
`; diff --git a/src/utils/create-slots.tsx b/src/utils/create-slots.tsx index 9489a43c1d9..2a1b51eec7c 100644 --- a/src/utils/create-slots.tsx +++ b/src/utils/create-slots.tsx @@ -1,5 +1,6 @@ import React from 'react' import {useForceUpdate} from './use-force-update' +import useLayoutEffect from './useIsomorphicLayoutEffect' /** createSlots is a factory that can create a * typesafe Slots + Slot pair to use in a component definition @@ -42,7 +43,7 @@ const createSlots = (slotNames: SlotNames[]) => { const [isMounted, setIsMounted] = React.useState(false) // fires after all the effects in children - React.useEffect(() => { + useLayoutEffect(() => { rerenderWithSlots() setIsMounted(true) }, [rerenderWithSlots]) @@ -86,7 +87,7 @@ const createSlots = (slotNames: SlotNames[]) => { > = ({name, children}) => { const {registerSlot, unregisterSlot, context} = React.useContext(SlotsContext) - React.useEffect(() => { + useLayoutEffect(() => { registerSlot(name, typeof children === 'function' ? children(context) : children) return () => unregisterSlot(name) }, [name, children, registerSlot, unregisterSlot, context])