Skip to content
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.

Sign up for GitHub

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

refactor: use transient props in various styled components #5093

Merged
merged 1 commit into from
Oct 31, 2023
Merged

refactor: use transient props in various styled components #5093

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/sanity/src/core/changeIndicators/ChangeIndicator.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ const ChangeBarWrapper = memo(function ChangeBarWrapper(
return (
<div {...restProps} ref={ref} onMouseEnter={onMouseEnter} onMouseLeave={onMouseLeave}>
<ElementWithChangeBar
hasFocus={hasFocus}
isChanged={isChanged}
disabled={disabled}
withHoverEffect={withHoverEffect}
Expand Down
74 changes: 37 additions & 37 deletions packages/sanity/src/core/changeIndicators/ElementWithChangeBar.styled.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,54 +6,54 @@ interface ThemeContext {
}

interface RootProps {
focus: boolean
hover: boolean
Comment on lines -9 to -10
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were removed as the styled component wasn't doing anything with these values

changed?: boolean
isReviewChangeOpen: boolean
disabled?: boolean
$changed?: boolean
$disabled?: boolean
$isReviewChangeOpen: boolean
$withHoverEffect?: boolean
}

const animationSpeed = 250

export const ChangeBarWrapper = styled.div<RootProps>(({changed, disabled, isReviewChangeOpen}) => {
if (disabled)
return css`
${ChangeBar} {
display: none;
}
`
export const ChangeBarWrapper = styled.div<RootProps>(
({$changed, $disabled, $isReviewChangeOpen}) => {
if ($disabled)
return css`
${ChangeBar} {
display: none;
}
`

return css`
--change-bar-offset: 2px;
return css`
--change-bar-offset: 2px;

display: flex;
position: relative;
display: flex;
position: relative;

@media (hover: hover) {
&:hover {
z-index: 10;
@media (hover: hover) {
&:hover {
z-index: 10;
}
}
}

/* hide when field is not changed */
${!changed &&
css`
${ChangeBar} {
opacity: 0;
pointer-events: none;
}
`}
/* hide when field is not changed */
${!$changed &&
css`
${ChangeBar} {
opacity: 0;
pointer-events: none;
}
`}

/* hide hover effect when review changes is open */
${isReviewChangeOpen &&
css`
${ChangeBarButton} {
opacity: 0;
}
`}
`
})
/* hide hover effect when review changes is open */
${$isReviewChangeOpen &&
css`
${ChangeBarButton} {
opacity: 0;
}
`}
`
},
)

export const FieldWrapper = styled.div`
flex-grow: 1;
Expand Down
36 changes: 9 additions & 27 deletions packages/sanity/src/core/changeIndicators/ElementWithChangeBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,30 +1,25 @@
import {useLayer} from '@sanity/ui'
import React, {useCallback, useMemo, useState} from 'react'
import React, {useMemo} from 'react'
import {ConnectorContext} from './ConnectorContext'
import {
ChangeBarWrapper,
FieldWrapper,
ChangeBar,
ChangeBarMarker,
ChangeBarButton,
ChangeBarMarker,
ChangeBarWrapper,
FieldWrapper,
} from './ElementWithChangeBar.styled'

export function ElementWithChangeBar(props: {
children: React.ReactNode
disabled?: boolean
hasFocus: boolean
isChanged?: boolean
withHoverEffect?: boolean
}) {
const {children, disabled, hasFocus, isChanged, withHoverEffect = true} = props
const {children, disabled, isChanged, withHoverEffect = true} = props

const [hover, setHover] = useState(false)
const {onOpenReviewChanges, isReviewChangesOpen} = React.useContext(ConnectorContext)
const {zIndex} = useLayer()

const handleMouseEnter = useCallback(() => setHover(true), [])
const handleMouseLeave = useCallback(() => setHover(false), [])

const changeBar = useMemo(
() =>
disabled || !isChanged ? null : (
Expand All @@ -35,34 +30,21 @@ export function ElementWithChangeBar(props: {
aria-label="Review changes"
data-testid="change-bar__button"
onClick={isReviewChangesOpen ? undefined : onOpenReviewChanges}
onMouseEnter={handleMouseEnter}
onMouseLeave={handleMouseLeave}
Comment on lines -38 to -39
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, this internal state has been removed as ChangeBarWrapper isn't varying on the value of hover

tabIndex={-1}
type="button"
$withHoverEffect={withHoverEffect}
/>
</ChangeBar>
),
[
disabled,
isChanged,
zIndex,
isReviewChangesOpen,
onOpenReviewChanges,
handleMouseEnter,
handleMouseLeave,
withHoverEffect,
],
[disabled, isChanged, zIndex, isReviewChangesOpen, onOpenReviewChanges, withHoverEffect],
)

return (
<ChangeBarWrapper
changed={isChanged}
data-testid="change-bar-wrapper"
disabled={disabled}
focus={hasFocus}
hover={hover}
isReviewChangeOpen={isReviewChangesOpen}
$changed={isChanged}
$disabled={disabled}
$isReviewChangeOpen={isReviewChangesOpen}
>
<FieldWrapper data-testid="change-bar__field-wrapper">{children}</FieldWrapper>
{changeBar}
Expand Down
10 changes: 3 additions & 7 deletions packages/sanity/src/core/changeIndicators/__workshop__/ChangeBarStory.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,16 @@
import {Container, Flex, TextArea} from '@sanity/ui'
import {useBoolean} from '@sanity/ui-workshop'
import React, {useCallback, useState} from 'react'
import React from 'react'
import {ElementWithChangeBar} from '../ElementWithChangeBar'

export default function ChangeBarStory() {
const isChanged = useBoolean('Changed', true, 'Props')
const [focused, setFocused] = useState(false)

const handleBlur = useCallback(() => setFocused(false), [])
const handleFocus = useCallback(() => setFocused(true), [])

return (
<Flex align="center" height="fill" justify="center" padding={4} sizing="border">
<Container width={0}>
<ElementWithChangeBar isChanged={isChanged} hasFocus={focused}>
<TextArea onBlur={handleBlur} onFocus={handleFocus} rows={5} />
<ElementWithChangeBar isChanged={isChanged}>
<TextArea rows={5} />
</ElementWithChangeBar>
</Container>
</Flex>
Expand Down
8 changes: 1 addition & 7 deletions packages/sanity/src/core/field/diff/components/DiffTooltip.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,7 @@ function DiffTooltipWithAnnotation(props: DiffTooltipWithAnnotationsProps) {

return (
<LegacyLayerProvider zOffset="paneFooter">
<Tooltip
content={content}
// data-placement={restProps.placement}
portal
allowedAutoPlacements={['top', 'bottom']}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed as it's now deprecated in Sanity UI, and including it would yield similar transient prop warnings

{...restProps}
>
<Tooltip content={content} portal {...restProps}>
{children}
</Tooltip>
</LegacyLayerProvider>
Expand Down
11 changes: 9 additions & 2 deletions packages/sanity/src/core/form/inputs/ReferenceInput/ReferenceLinkCard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,14 @@ export const ReferenceLinkCard = forwardRef(function ReferenceLinkCard(
props: ReferenceLinkCardProps & React.HTMLProps<HTMLElement>,
ref: React.ForwardedRef<HTMLElement>,
) {
const {documentType, as: asProp, ...restProps} = props
const {
as: asProp,
// We exclude `documentId` from spread props to avoid passing it to the Card component
// eslint-disable-next-line @typescript-eslint/no-unused-vars
documentId,
Comment on lines +34 to +36
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would love to know if there's a more elegant way to do this!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should be able to assingn it to _ and have eslint ignore it, e.g. documentId: _

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered this! Though I believe we need to amend our eslint config slightly to accommodate, e.g.

'@typescript-eslint/no-unused-vars': [
  'warn',
  {
    varsIgnorePattern: '^_',
  },
],

Happy to add if you feel appropriate! Otherwise we can just leave as-is

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since const {foo: _, bar: _, ...rest} = … will yell at you for redeclaring consts, will that rule allow omitting more than one property? Maybe we could configure it to be _ignore_${propertyName} (or maybe that's too much?).

There's also no-unused-vars#ignorerestsiblings, which is more specific to this particular case.

documentType,
...cardProps
} = props
const dataAs = documentType ? 'a' : undefined

// If the child link is clicked without a document type, an error will be thrown.
Expand All @@ -39,7 +46,7 @@ export const ReferenceLinkCard = forwardRef(function ReferenceLinkCard(

return (
<StyledCard
{...restProps}
{...cardProps}
data-as={dataAs}
documentType={documentType}
forwardedAs={as}
Expand Down
16 changes: 8 additions & 8 deletions packages/sanity/src/core/form/inputs/files/ImageInput/ImagePreview.styled.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,21 @@ export const SpinnerWrapper = styled(Spinner)`
`

export const Overlay = styled(Flex)<{
tone: Exclude<CardTone, 'inherit'>
drag: boolean
}>(({tone, drag}) => {
const textColor = studioTheme.color.light[tone].card.enabled.fg
const backgroundColor = rgba(studioTheme.color.light[tone].card.enabled.bg, 0.8)
$drag: boolean
$tone: Exclude<CardTone, 'inherit'>
}>(({$drag, $tone}) => {
const textColor = studioTheme.color.light[$tone].card.enabled.fg
const backgroundColor = rgba(studioTheme.color.light[$tone].card.enabled.bg, 0.8)

return css`
position: absolute;
top: 0;
left: 0;
right: 0;
bottom: 0;
backdrop-filter: ${drag ? 'blur(10px)' : ''};
color: ${tone ? textColor : ''};
background-color: ${drag ? backgroundColor : 'transparent'};
backdrop-filter: ${$drag ? 'blur(10px)' : ''};
color: ${$tone ? textColor : ''};
background-color: ${$drag ? backgroundColor : 'transparent'};
`
})

Expand Down
2 changes: 1 addition & 1 deletion packages/sanity/src/core/form/inputs/files/ImageInput/ImagePreview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ function OverlayComponent({
content: React.ReactNode
}) {
return (
<Overlay justify="flex-end" padding={3} tone={cardTone} drag={drag}>
<Overlay justify="flex-end" padding={3} $drag={drag} $tone={cardTone}>
<FlexOverlay direction="column" align="center" justify="center">
{content}
</FlexOverlay>
Expand Down
4 changes: 2 additions & 2 deletions packages/sanity/src/desk/panes/document/documentPanel/header/DocumentHeaderTabs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,14 @@ function DocumentHeaderTab(props: {
tabPanelId: string
viewId: string | null
}) {
const {isActive, tabPanelId, viewId} = props
const {isActive, tabPanelId, viewId, ...rest} = props
const {ready} = useDocumentPane()
const {setView} = usePaneRouter()
const handleClick = useCallback(() => setView(viewId), [setView, viewId])

return (
<Tab
{...props} // required to enable <TabList> keyboard navigation
{...rest} // required to enable <TabList> keyboard navigation
aria-controls={tabPanelId}
disabled={!ready}
fontSize={1}
Expand Down
Loading