-
Notifications
You must be signed in to change notification settings - Fork 63
feat(component): add enhancements to panel component #1678
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| '@bigcommerce/big-design': minor | ||
| '@bigcommerce/docs': minor | ||
| --- | ||
|
|
||
| Enhanced Panel component with extra features like Lozenge, Dropdown action and a new PanelContents component |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| import React, { memo, ReactNode } from 'react'; | ||
|
|
||
| import { BoxProps } from '../../Box'; | ||
|
|
||
| import { StyledPanelContents, StyledPanelContentsWrapper } from './styled'; | ||
|
|
||
| export interface PanelContentProps extends BoxProps { | ||
| children?: ReactNode; | ||
| padded?: boolean; | ||
| height?: string; | ||
| scrollable?: boolean; | ||
| } | ||
|
|
||
| export const PanelContents: React.FC<PanelContentProps> = memo(({ children, ...props }) => { | ||
| return ( | ||
| <StyledPanelContentsWrapper {...props}> | ||
| <StyledPanelContents {...props}>{children}</StyledPanelContents> | ||
| </StyledPanelContentsWrapper> | ||
| ); | ||
| }); | ||
|
|
||
| PanelContents.displayName = 'PanelContents'; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| // Jest Snapshot v1, https://goo.gl/fbAQLP | ||
|
|
||
| exports[`PanelContents renders StyledPanelContents with correct props 1`] = ` | ||
| <div | ||
| class="styled__StyledBox-sc-12d4dbf-0 imudcb styled__StyledPanelContents-sc-dd60c8b6-1 fnisvI" | ||
| height="auto" | ||
| > | ||
| Test string | ||
| </div> | ||
| `; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export { PanelContents, type PanelContentProps } from './Contents'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| import { theme } from '@bigcommerce/big-design-theme'; | ||
| import { render } from '@testing-library/react'; | ||
| import React from 'react'; | ||
|
|
||
| import { PanelContents } from './Contents'; | ||
|
|
||
| describe('PanelContents', () => { | ||
| it('renders children correctly', () => { | ||
| const { getByText } = render( | ||
| <PanelContents> | ||
| <div>Test Content</div> | ||
| </PanelContents>, | ||
| ); | ||
|
|
||
| expect(getByText('Test Content')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('applies default props to StyledPanelContentsWrapper', () => { | ||
| const { container } = render(<PanelContents />); | ||
| const wrapper = container.firstChild; | ||
|
|
||
| expect(wrapper).toHaveStyle('height: auto'); | ||
| }); | ||
|
|
||
| it('applies the `padded` prop correctly', () => { | ||
| const { container } = render(<PanelContents padded={false} />); | ||
| const wrapper = container.firstChild; | ||
|
|
||
| expect(wrapper).toHaveStyle(`margin-inline: -${theme.spacing.medium}`); | ||
| }); | ||
|
|
||
| it('applies the `height` prop correctly', () => { | ||
| const { container } = render(<PanelContents height="200px">Test string</PanelContents>); | ||
| const wrapper = container.firstChild; | ||
|
|
||
| expect(wrapper).toHaveStyle('height: 200px'); | ||
| }); | ||
|
|
||
| it('applies the `scrollable` prop correctly', () => { | ||
| const { container } = render(<PanelContents scrollable>Test string</PanelContents>); | ||
| const wrapper = container.firstChild; | ||
|
|
||
| expect(wrapper).toHaveStyle('overflow: auto'); | ||
| }); | ||
|
|
||
| it('renders StyledPanelContents with correct props', () => { | ||
| const { getByText } = render(<PanelContents scrollable>Test string</PanelContents>); | ||
| const contents = getByText('Test string'); | ||
|
|
||
| expect(contents).toBeInTheDocument(); | ||
| expect(contents).toMatchSnapshot(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| import { theme as defaultTheme } from '@bigcommerce/big-design-theme'; | ||
| import styled, { css } from 'styled-components'; | ||
|
|
||
| import { Box } from '../../Box'; | ||
|
|
||
| import { PanelContentProps } from './Contents'; | ||
|
|
||
| export const StyledPanelContentsWrapper = styled(Box)<Omit<PanelContentProps, 'children'>>` | ||
| ${({ theme, scrollable }) => | ||
| scrollable && | ||
| css` | ||
| background-image: linear-gradient( | ||
| 0deg, | ||
| ${theme.colors.secondary40} 0%, | ||
| transparent 1px, | ||
| transparent 100% | ||
| ), | ||
| linear-gradient(180deg, ${theme.colors.secondary40} 0%, transparent 1px, transparent 100%); | ||
| overflow: auto; | ||
| `} | ||
| ${({ height }) => | ||
| height && | ||
| css` | ||
| height: ${height}; | ||
| `} | ||
| ${({ theme, padded }) => | ||
| padded !== true && | ||
|
Contributor
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. we normally just do negative boolean checks like |
||
| css` | ||
| margin-inline: -${theme.spacing.medium}; | ||
| `} | ||
| ${({ theme }) => theme.breakpoints.tablet} { | ||
| ${({ theme, padded }) => | ||
| padded !== true && | ||
| css` | ||
| margin-inline: -${theme.spacing.xLarge}; | ||
|
Contributor
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. these minus margins are a bit awkward both the technique, plus they're trying to undo the Panel's padding, without referencing the Panel's padding value I'm also guessing that we wouldn't want e.g. <Panel
header="Panel header"
fullWidthContents
>
<Text>
Hello there
</Text>
</Panel>or with a <Panel
header="Panel header"
contents={{ fullWidth: true }}
>
<Text>
Hello there
</Text>
</Panel> |
||
| `} | ||
| } | ||
| `; | ||
| StyledPanelContentsWrapper.defaultProps = { | ||
| theme: defaultTheme, | ||
| padded: true, | ||
| scrollable: false, | ||
| height: 'auto', | ||
| }; | ||
|
|
||
| export const StyledPanelContents = styled(Box)<Omit<PanelContentProps, 'children'>>` | ||
| min-height: 100%; | ||
| ${({ theme, scrollable }) => | ||
| scrollable && | ||
| css` | ||
| border-block-start: ${theme.border.box}; | ||
| border-block-end: ${theme.border.box}; | ||
| border-block-start-color: ${theme.colors.white}; | ||
| border-block-end-color: ${theme.colors.white}; | ||
| `} | ||
| `; | ||
| StyledPanelContents.defaultProps = { | ||
| theme: defaultTheme, | ||
| padded: true, | ||
| scrollable: false, | ||
| height: 'auto', | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,14 @@ | ||
| import React, { ComponentPropsWithoutRef, forwardRef, isValidElement, memo, Ref } from 'react'; | ||
|
|
||
| import { MarginProps } from '../../helpers'; | ||
| import { excludeMarginProps, MarginProps } from '../../helpers'; | ||
| import { excludePaddingProps } from '../../helpers/paddings/paddings'; | ||
| import { warning } from '../../utils'; | ||
| import { Badge, BadgeProps } from '../Badge/Badge'; | ||
| import { Box } from '../Box'; | ||
| import { Button, ButtonProps } from '../Button'; | ||
| import { Dropdown, DropdownProps } from '../Dropdown'; | ||
| import { Flex } from '../Flex'; | ||
| import { Lozenge, LozengeProps } from '../Lozenge'; | ||
| import { Text } from '../Typography'; | ||
|
|
||
| import { StyledH2, StyledPanel } from './styled'; | ||
|
|
@@ -15,22 +17,56 @@ interface PrivateProps { | |
| forwardedRef: Ref<HTMLDivElement>; | ||
| } | ||
|
|
||
| export interface PanelAction extends Omit<ButtonProps, 'children'> { | ||
| export interface PanelActionProps extends Omit<ButtonProps, 'children' | 'mobileWidth'> { | ||
| text?: string; | ||
| } | ||
|
|
||
| export interface PanelDropdownProps extends Omit<DropdownProps, 'toggle'> { | ||
| toggle: PanelActionProps; | ||
| } | ||
|
Comment on lines
+24
to
+26
Contributor
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. why not just stick to |
||
|
|
||
| export type PanelAction = PanelActionProps | PanelDropdownProps; | ||
|
|
||
| export interface PanelProps extends ComponentPropsWithoutRef<'div'>, MarginProps { | ||
| children?: React.ReactNode; | ||
| description?: React.ReactNode; | ||
| header?: string; | ||
| headerId?: string; | ||
| action?: PanelAction; | ||
| badge?: BadgeProps; | ||
| lozenge?: LozengeProps; | ||
| } | ||
|
|
||
| const Action = (action: PanelAction) => { | ||
| if ('toggle' in action) { | ||
| const { toggle, ...dropdownProps } = action; | ||
| const { text, ...buttonProps } = toggle; | ||
|
|
||
| return ( | ||
| <Dropdown | ||
| {...dropdownProps} | ||
| toggle={ | ||
| <Button {...excludeMarginProps(buttonProps)} mobileWidth="auto"> | ||
| {text} | ||
| </Button> | ||
| } | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| const { text, ...buttonProps } = action; | ||
|
|
||
| return ( | ||
| <Button {...excludeMarginProps(buttonProps)} mobileWidth="auto"> | ||
| {text} | ||
| </Button> | ||
| ); | ||
| }; | ||
|
|
||
| export const RawPanel: React.FC<PanelProps & PrivateProps> = memo(({ forwardedRef, ...props }) => { | ||
| const filteredProps = excludePaddingProps(props); | ||
| const { action, children, description, header, headerId, badge, ...rest } = filteredProps; | ||
| const { action, children, description, header, headerId, badge, lozenge, ...rest } = | ||
| filteredProps; | ||
|
|
||
| const renderHeader = () => { | ||
| if (!header && !action) { | ||
|
|
@@ -42,14 +78,17 @@ export const RawPanel: React.FC<PanelProps & PrivateProps> = memo(({ forwardedRe | |
| } | ||
|
|
||
| return ( | ||
| <Flex flexDirection="row"> | ||
| <Flex alignItems="center" flexDirection="row" flexGap="0.5rem" justifyContent="space-between"> | ||
|
Contributor
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. anything we can do with these |
||
| {Boolean(header) && ( | ||
| <StyledH2 id={headerId} marginBottom="none"> | ||
| {header} | ||
| {badge && <Badge marginLeft="xSmall" {...badge} />} | ||
| </StyledH2> | ||
| <Flex alignItems="center" flexDirection="row" flexGap="0.5rem"> | ||
| <StyledH2 id={headerId} marginBottom="none"> | ||
| {header} | ||
| </StyledH2> | ||
| {lozenge && <Lozenge {...lozenge} />} | ||
| {badge && <Badge {...badge} />} | ||
| </Flex> | ||
| )} | ||
| {action && <Button {...action}>{action.text}</Button>} | ||
| {action && <Action {...action} />} | ||
| </Flex> | ||
| ); | ||
| }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,2 @@ | ||
| export { Panel, type PanelProps } from './Panel'; | ||
| export { PanelContents, type PanelContentProps } from './Contents'; |
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.
would we ever want to support a scenario where the height is fixed, but scrolling is disabled?
would this just result in content being abruptly cropped and unreachable?