{"commit":{"subject":"Removed unnecessary layout effect in favor of a setState","authorName":"Brian Vaughn","revision":"2ac815784b9aff97fb367d605c126d55846cecc7","authorEmail":"bvaughn@fb.com","authoredAt":"2019-04-07T15:19:17-07:00","shortRevision":"2ac815784b"},"body":"","parents":["2e5e88e127f5bac959aef7c0536b9d0cecd746ab"],"patch":"diff --git a\/src\/devtools\/views\/Components\/OwnersStack.css b\/src\/devtools\/views\/Components\/OwnersStack.css\nindex d2ebdde61e..270e22ee3b 100644\n--- a\/src\/devtools\/views\/Components\/OwnersStack.css\n+++ b\/src\/devtools\/views\/Components\/OwnersStack.css\n@@ -5,7 +5,7 @@\n }\n \n .Component,\n-.FocusedComponent {\n+.SelectedComponent {\n padding: 0.25rem;\n margin-right: 0.5rem;\n color: var(--color-component-name);\n@@ -14,11 +14,8 @@\n border-radius: 0.125rem;\n border: none;\n background: none;\n-}\n-\n-.Component {\n cursor: pointer;\n- color: var(--color-component-name);\n+ text-align: left;\n }\n .Component:hover {\n background-color: var(--color-hover-background);\n@@ -28,52 +25,38 @@\n background-color: var(--color-hover-background);\n }\n \n-.FocusedComponent {\n+.SelectedComponent {\n background-color: var(--color-selected-background);\n color: var(--color-selected-foreground);\n }\n-.FocusedComponent:focus {\n+.SelectedComponent:focus {\n outline: none;\n }\n \n-.ElementsBar {\n+.Bar {\n+ display: flex;\n flex: 1 0 auto;\n }\n \n-.ElementsBarSelectedOnly {\n- margin-left: 0.25rem;\n-}\n-\n-.ElementsBarSelectedOnly .Component {\n- visibility: hidden;\n-}\n-.ElementsBarSelectedOnly .FocusedComponent {\n- float: left;\n+.Toggle {\n+ margin-right: 0.5rem;\n }\n \n-.ElementsDropdown {\n+.DropdownWrapper {\n+ display: inline-block;\n position: relative;\n }\n \n .Dropdown {\n z-index: 1;\n position: absolute;\n- top: calc(100% + 5px);\n+ top: calc(100% + 0.25rem);\n left: 0;\n background-color: var(--color-background);\n- border: 1px solid var(--color-selected-border);\n overflow-y: auto;\n-}\n-\n-.Dropdown .Component,\n-.Dropdown .FocusedComponent {\n- display: block;\n- margin: 0.25rem 0.75rem 0.35rem;\n-}\n-\n-.DropdownButtonActive {\n- background-color: var(--color-selected-background);\n- color: var(--color-selected-foreground);\n+ max-height: 10rem;\n+ display: flex;\n+ flex-direction: column;\n }\n \n .VRule {\ndiff --git a\/src\/devtools\/views\/Components\/OwnersStack.js b\/src\/devtools\/views\/Components\/OwnersStack.js\nindex f3888c1817..9bf7c57732 100644\n--- a\/src\/devtools\/views\/Components\/OwnersStack.js\n+++ b\/src\/devtools\/views\/Components\/OwnersStack.js\n@@ -3,13 +3,12 @@ import React, {\n useCallback,\n useContext,\n useLayoutEffect,\n+ useRef,\n useState,\n- createRef,\n- forwardRef,\n } from 'react';\n-import classNames from 'classnames';\n import Button from '..\/Button';\n import ButtonIcon from '..\/ButtonIcon';\n+import Toggle from '..\/Toggle';\n import { TreeContext } from '.\/TreeContext';\n import { StoreContext } from '..\/context';\n import { useIsOverflowing } from '..\/hooks';\n@@ -18,67 +17,129 @@ import type { Element } from '.\/types';\n \n import styles from '.\/OwnersStack.css';\n \n+export default function OwnerStack() {\n+ const { ownerStack, ownerStackIndex, resetOwnerStack } = useContext(\n+ TreeContext\n+ );\n+\n+ const [elementsTotalWidth, setElementsTotalWidth] = useState(0);\n+ const elementsBarRef = useRef(null);\n+ const isOverflowing = useIsOverflowing(elementsBarRef, elementsTotalWidth);\n+\n+ useLayoutEffect(() => {\n+ if (elementsBarRef.current === null) {\n+ return () => {};\n+ }\n+\n+ let elementsTotalWidth = 0;\n+ for (let i = 0; i < ownerStack.length; i++) {\n+ const element = elementsBarRef.current.children[i];\n+ const computedStyle = getComputedStyle(element);\n+\n+ elementsTotalWidth +=\n+ element.offsetWidth +\n+ parseInt(computedStyle.marginLeft, 10) +\n+ parseInt(computedStyle.marginRight, 10);\n+ }\n+\n+ setElementsTotalWidth(elementsTotalWidth);\n+ }, [elementsBarRef, ownerStack.length]);\n+\n+ return (\n+

\n+ \n+ \n+ <\/Button>\n+

\n+

\n+ {isOverflowing && (\n+ \n+ )}\n+ {isOverflowing ? (\n+ \n+ ) : (\n+ ownerStack.map((id, index) => (\n+ \n+ ))\n+ )}\n+ <\/div>\n+ <\/div>\n+ );\n+}\n+\n type ElementsDropdownProps = {\n- selectedElementIndex: number | null,\n- children: Array,\n+ ownerStack: Array,\n+ ownerStackIndex: number | null,\n };\n function ElementsDropdown({\n- selectedElementIndex,\n- children,\n+ ownerStack,\n+ ownerStackIndex,\n }: ElementsDropdownProps) {\n+ const store = useContext(StoreContext);\n+ const { selectOwner } = useContext(TreeContext);\n+\n const [isDropdownVisible, setIsDropdownVisible] = useState(false);\n \n- const handleClick = useCallback(() => {\n+ const handleDropdownButtonClick = useCallback(() => {\n setIsDropdownVisible(!isDropdownVisible);\n }, [isDropdownVisible, setIsDropdownVisible]);\n \n- useLayoutEffect(() => {\n- setIsDropdownVisible(false);\n- \/\/ eslint-disable-next-line react-hooks\/exhaustive-deps\n- }, [selectedElementIndex]);\n+ const handleElementClick = useCallback(\n+ (id: number) => {\n+ selectOwner(id);\n+ setIsDropdownVisible(false);\n+ },\n+ [selectOwner, setIsDropdownVisible]\n+ );\n \n return (\n-

\n- \n+ \n \n- <\/Button>\n- {isDropdownVisible &&

{children}<\/div>}\n+ <\/Toggle>\n+ {isDropdownVisible && (\n+

\n+ {ownerStack.map((id, index) => (\n+ handleElementClick(id)}\n+ >\n+ {((store.getElementByID(id): any): Element).displayName}\n+ <\/button>\n+ ))}\n+ <\/div>\n+ )}\n <\/div>\n );\n }\n \n-type ElementsBarProps = {\n- elements: Array,\n- showSelectedOnly: boolean,\n-};\n-const ElementsBar = forwardRef(\n- ({ elements, showSelectedOnly }: ElementsBarProps, ref: Object) => {\n- return (\n- \n- {elements}\n- <\/div>\n- );\n- }\n-);\n-\n type ElementViewProps = {\n id: number,\n- index: number,\n+ index: number | null,\n };\n function ElementView({ id, index }: ElementViewProps) {\n- const { ownerStackIndex, selectOwner } = useContext(TreeContext);\n const store = useContext(StoreContext);\n+ const { ownerStackIndex, selectOwner } = useContext(TreeContext);\n+\n const { displayName } = ((store.getElementByID(id): any): Element);\n \n const isSelected = ownerStackIndex === index;\n@@ -91,66 +152,10 @@ function ElementView({ id, index }: ElementViewProps) {\n \n return (\n \n {displayName}\n <\/button>\n );\n }\n-\n-export default function OwnerStack() {\n- const { ownerStack, ownerStackIndex, resetOwnerStack } = useContext(\n- TreeContext\n- );\n-\n- const [elementsTotalWidth, setElementsTotalWidth] = useState(0);\n- const elementsBarRef = createRef();\n- const isOverflowing = useIsOverflowing(elementsBarRef, elementsTotalWidth);\n-\n- const elements = ownerStack.map((id, index) => (\n- \n- ));\n-\n- useLayoutEffect(() => {\n- if (elementsBarRef.current === null) {\n- return () => {};\n- }\n-\n- let elementsTotalWidth = 0;\n- for (let i = 0; i < ownerStack.length; i++) {\n- const element = elementsBarRef.current.children[i];\n- const computedStyle = getComputedStyle(element);\n-\n- elementsTotalWidth +=\n- element.offsetWidth +\n- parseInt(computedStyle.marginLeft, 10) +\n- parseInt(computedStyle.marginRight, 10);\n- }\n-\n- setElementsTotalWidth(elementsTotalWidth);\n- }, [elementsBarRef, ownerStack.length]);\n-\n- return (\n-

\n- \n- \n- <\/Button>\n- {isOverflowing && (\n- \n- {elements}\n- <\/ElementsDropdown>\n- )}\n-

\n- \n- <\/div>\n- );\n-}\n","files":[{"path":"src\/devtools\/views\/Components\/OwnersStack.css","status":"M"},{"path":"src\/devtools\/views\/Components\/OwnersStack.js","status":"M"}]}

dillon1000/react

Files changed