-
Notifications
You must be signed in to change notification settings - Fork 322
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(Responsive list): migrate to typescript #2127
Merged
rivka-ungar
merged 8 commits into
master
from
ResponsiveList----migrate-to-Typescript-4247428543
May 19, 2024
+46
−64
Merged
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
5a26c4a
refactor(ResponsiveList): migrate to Typescript
rivka-ungar 96c6331
migrate to typescript
rivka-ungar 5c6b190
migrate to typescript
rivka-ungar d4f7adc
Merge branch 'master' into ResponsiveList----migrate-to-Typescript-42…
rivka-ungar 37128b6
Merge branch 'master' into ResponsiveList----migrate-to-Typescript-42…
rivka-ungar 4cab8e2
cr fixes
rivka-ungar 0048775
Merge remote-tracking branch 'origin/ResponsiveList----migrate-to-Typ…
rivka-ungar 00acb85
fix test
rivka-ungar File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,37 +1,35 @@ | ||
import cx from "classnames"; | ||
import React, { useRef, forwardRef, useMemo } from "react"; | ||
import PropTypes from "prop-types"; | ||
import useMergeRef from "../../hooks/useMergeRef"; | ||
import MenuButton from "../MenuButton/MenuButton"; | ||
import useElementsOverflowingIndex from "../../hooks/useElementsOverflowingIndex"; | ||
import { ComponentDefaultTestId, getTestId } from "../../tests/test-ids-utils"; | ||
import styles from "./ResponsiveList.module.scss"; | ||
import { VibeComponent } from "../../types"; | ||
import MenuButton from "../MenuButton/MenuButton"; | ||
import { DEFAULT_MINIMAL_MARGIN, EMPTY_ARRAY, ResponsiveListProps } from "./ResponsiveList.types"; | ||
|
||
const DEFAULT_MINIMAL_MARGIN = 32; | ||
const EMPTY_ARRAY = []; | ||
|
||
const ResponsiveList = forwardRef( | ||
const ResponsiveList: VibeComponent<ResponsiveListProps> = forwardRef<HTMLDivElement, ResponsiveListProps>( | ||
( | ||
{ | ||
id, | ||
className, | ||
rootClassName, | ||
id = "", | ||
className = "", | ||
rootClassName = "", | ||
children, | ||
menuButtonSize, | ||
paddingSize, | ||
dialogZIndex, | ||
dialogClassName, | ||
menuButtonClassName, | ||
menuWrapperClassName, | ||
resizeDebounceTime, | ||
menuButtonAriaLabel, | ||
menuButtonProps, | ||
menuButtonSize = MenuButton.sizes.SMALL, | ||
paddingSize = DEFAULT_MINIMAL_MARGIN, | ||
dialogZIndex = 9999, | ||
dialogClassName = "", | ||
menuButtonClassName = "", | ||
menuWrapperClassName = "", | ||
resizeDebounceTime = 0, | ||
menuButtonAriaLabel = "More Actions", | ||
menuButtonProps = {}, | ||
"data-testid": dataTestId | ||
}, | ||
ref | ||
) => { | ||
const componentRef = useRef(null); | ||
const mergedRef = useMergeRef(ref, componentRef); | ||
const componentRef = useRef<HTMLDivElement>(null); | ||
const mergedRef = useMergeRef<HTMLDivElement>(ref, componentRef); | ||
const index = useElementsOverflowingIndex({ | ||
ref: componentRef, | ||
children, | ||
|
@@ -58,7 +56,7 @@ const ResponsiveList = forwardRef( | |
}, [children, index]); | ||
|
||
const hiddenChildren = useMemo(() => { | ||
const childrenArray = React.Children.toArray(children); | ||
const childrenArray = React.Children.toArray(children) as React.ReactElement[]; | ||
talkor marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
return childrenArray.map(el => el?.props?.responsiveListPlaceholder || el); | ||
}, [children]); | ||
|
@@ -74,7 +72,7 @@ const ResponsiveList = forwardRef( | |
{directChildren} | ||
{!!menuChildren.length && ( | ||
<MenuButton | ||
componentClassName={cx(styles.listMenuButton, menuButtonClassName)} | ||
className={cx(styles.listMenuButton, menuButtonClassName)} | ||
talkor marked this conversation as resolved.
Show resolved
Hide resolved
|
||
size={menuButtonSize} | ||
openDialogComponentClassName={cx(styles.menuButtonDialog, dialogClassName)} | ||
zIndex={dialogZIndex} | ||
|
@@ -89,7 +87,7 @@ const ResponsiveList = forwardRef( | |
<div ref={mergedRef} className={cx(styles.responsiveList, styles.dummy, className)}> | ||
{hiddenChildren} | ||
<MenuButton | ||
componentClassName={cx(styles.listMenuButton, menuButtonClassName)} | ||
className={cx(styles.listMenuButton, menuButtonClassName)} | ||
size={menuButtonSize} | ||
openDialogComponentClassName={cx(styles.menuButtonDialog, dialogClassName)} | ||
zIndex={dialogZIndex} | ||
|
@@ -103,44 +101,4 @@ const ResponsiveList = forwardRef( | |
); | ||
} | ||
); | ||
|
||
ResponsiveList.menuButtonSizes = MenuButton.sizes; | ||
ResponsiveList.propTypes = { | ||
id: PropTypes.string, | ||
className: PropTypes.string, | ||
menuButtonClassName: PropTypes.string, | ||
menuWrapperClassName: PropTypes.string, | ||
/** | ||
These attributes will be passed to the MenuButton | ||
*/ | ||
Comment on lines
-113
to
-115
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's keep those JSDoc comments as they help documenting the props |
||
menuButtonProps: PropTypes.object, | ||
menuButtonAriaLabel: PropTypes.string, | ||
rootClassName: PropTypes.string, | ||
dialogClassName: PropTypes.string, | ||
menuButtonSize: PropTypes.oneOf(Object.values(ResponsiveList.menuButtonSizes)), | ||
/** | ||
Amount of space to save between the menu button and the content | ||
*/ | ||
paddingSize: PropTypes.number, | ||
dialogZIndex: PropTypes.number, | ||
/** | ||
* we use resize observer behind the scene to update the size, debounce the amount of callbacks (each callback may cause a reflow) | ||
*/ | ||
resizeDebounceTime: PropTypes.number | ||
}; | ||
ResponsiveList.defaultProps = { | ||
id: "", | ||
className: "", | ||
dialogClassName: "", | ||
menuButtonClassName: "", | ||
menuWrapperClassName: "", | ||
rootClassName: "", | ||
menuButtonAriaLabel: "More Actions", | ||
menuButtonProps: {}, | ||
menuButtonSize: ResponsiveList.menuButtonSizes.SMALL, | ||
paddingSize: DEFAULT_MINIMAL_MARGIN, | ||
dialogZIndex: 9999, | ||
resizeDebounceTime: 0 | ||
}; | ||
|
||
export default ResponsiveList; |
21 changes: 21 additions & 0 deletions
21
packages/core/src/components/ResponsiveList/ResponsiveList.types.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
import { ReactNode } from "react"; | ||
import { VibeComponentProps } from "../../types"; | ||
import { MenuButtonSize } from "../MenuButton/MenuButtonConstants"; | ||
import { MenuButtonProps } from "../MenuButton/MenuButton"; | ||
|
||
export const DEFAULT_MINIMAL_MARGIN = 32; | ||
export const EMPTY_ARRAY: ReactNode[] = []; | ||
|
||
export interface ResponsiveListProps extends VibeComponentProps { | ||
rootClassName?: string; | ||
children?: ReactNode[]; | ||
menuButtonSize?: MenuButtonSize; | ||
paddingSize?: number; | ||
dialogZIndex?: number; | ||
dialogClassName?: string; | ||
menuButtonClassName?: string; | ||
menuWrapperClassName?: string; | ||
resizeDebounceTime?: number; | ||
menuButtonAriaLabel?: string; | ||
menuButtonProps?: MenuButtonProps; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These and the rest empty strings can just be undefined