Skip to content

Commit

Permalink
[FX-5908] Fix initial position of Slider (#4572)
Browse files Browse the repository at this point in the history
  • Loading branch information
TomasSlama authored Oct 10, 2024
1 parent dd142be commit 0dbab90
Show file tree
Hide file tree
Showing 14 changed files with 102 additions and 34 deletions.
8 changes: 8 additions & 0 deletions .changeset/thick-peaches-build.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@toptal/picasso-slider': patch
'@toptal/picasso': patch
---

### Slider

- fix initial position of Tooltip
5 changes: 5 additions & 0 deletions .changeset/tiny-news-compete.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@toptal/picasso-carousel': patch
---

- refactor useOnScreen usage after the breaking change
13 changes: 13 additions & 0 deletions .changeset/twelve-years-play.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
'@toptal/picasso-utils': major
'@toptal/picasso': major
---

### useOnScreen

- change return value of the hook to let component know when the oberver starts observing

```diff
-const isOnScreen = useOnScreen({...})
+const { isOnScreen, isObserved } = useOnScreen({...})
```
1 change: 1 addition & 0 deletions cypress/component/Slider.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ describe('Slider', () => {
it('renders range with tooltips intersect', () => {
cy.mount(<TestSlider value={[10, 11]} tooltipFormat={renderLabel} />)

cy.contains('GMT+10:00').should('be.visible')
cy.get('body').happoScreenshot({
component,
variant: 'range/when-tooltip-intersect',
Expand Down
25 changes: 25 additions & 0 deletions jest.setup.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,27 @@


// Mock for IntersectionObserver
class IntersectionObserverMock {
constructor(callback) {
this.callback = callback;
this.observables = [];
}

observe(element) {
this.observables.push(element);
this.callback([{ isIntersecting: true, target: element }]); // Customize as needed
}

unobserve(element) {
this.observables = this.observables.filter(obs => obs !== element);
}

disconnect() {
this.observables = [];
}
}

// Mock global IntersectionObserver
global.IntersectionObserver = IntersectionObserverMock;
global.TextEncoder = require('util').TextEncoder;
global.TextDecoder = require('util').TextDecoder;
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const useAutoplay = ({
rewind,
isLastPage,
}: Props) => {
const isOnScreen = useOnScreen({ ref: wrapperRef })
const { isOnScreen } = useOnScreen({ ref: wrapperRef })
const isMouseOver = useMouseEnter(wrapperRef)

useInterval({
Expand Down
21 changes: 12 additions & 9 deletions packages/base/Slider/src/Slider/Slider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,21 +84,24 @@ export const Slider = forwardRef<HTMLElement, Props>(function Slider(
} = props
const containerRef = useRef<HTMLDivElement>(null)
const sliderRef = useCombinedRefs<HTMLElement>(ref, useRef<HTMLElement>(null))
const { isPartiallyOverlapped, handleValueLabelOnRender } = useLabelOverlap({
value,
})

const isThumbHidden =
hideThumbOnEmpty && (typeof value === 'undefined' || value === null)

// The rootMargin is not working correctly in the storybooks iframe
// To test properly we can open the iframe in new window
const isContainerOnScreen = useOnScreen({
const { isOnScreen, isObserved } = useOnScreen({
ref: containerRef,
rootMargin: '-24px 0px 0px 0px',
threshold: 1,
})

const { isPartiallyOverlapped, handleValueLabelOnRender } = useLabelOverlap({
value,
// until IntersectionObserver starts observing the element, we don't render the tooltip
isTooltipRendered: isObserved,
})

const isThumbHidden =
hideThumbOnEmpty && (typeof value === 'undefined' || value === null)

return (
<div
ref={containerRef}
Expand Down Expand Up @@ -153,9 +156,9 @@ export const Slider = forwardRef<HTMLElement, Props>(function Slider(
),
},
valueLabel: {
tooltip,
tooltip: isObserved ? tooltip : 'off',
onRender: handleValueLabelOnRender,
yPlacement: isContainerOnScreen ? 'top' : 'bottom',
yPlacement: isOnScreen ? 'top' : 'bottom',
isOverlaped: isPartiallyOverlapped,
},
}}
Expand Down
4 changes: 2 additions & 2 deletions packages/base/Slider/src/Slider/__snapshots__/test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ exports[`Slider renders 1`] = `
value="0"
/>
<span
class="absolute will-change transition-transform hidden top-[calc(100%+2px)] left-[calc(100%-13px)]"
class="absolute will-change transition-transform hidden bottom-[calc(100%+2px)] left-[calc(100%-13px)]"
>
<span
class="shadow-4 text-sm text-white bg-graphite m-1 rounded-sm py-[2px] px-2 max-w break-words"
Expand Down Expand Up @@ -84,7 +84,7 @@ exports[`Slider with initial value 1`] = `
value="4"
/>
<span
class="absolute will-change transition-transform hidden top-[calc(100%+2px)] left-[calc(100%-13px)]"
class="absolute will-change transition-transform hidden bottom-[calc(100%+2px)] left-[calc(100%-13px)]"
>
<span
class="shadow-4 text-sm text-white bg-graphite m-1 rounded-sm py-[2px] px-2 max-w break-words"
Expand Down
18 changes: 15 additions & 3 deletions packages/base/Slider/src/Slider/hooks/use-label-overlap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,21 @@ import { useCallback, useEffect, useState } from 'react'

import { checkOverlap } from '../../utils'

export const useLabelOverlap = ({ value }: { value?: number | number[] }) => {
export const useLabelOverlap = ({
value,
isTooltipRendered,
}: {
value?: number | number[]
isTooltipRendered: boolean
}) => {
const [isPartiallyOverlapped, setIsPartiallyOverlapped] = useState(false)
const [valueLabels, setValueLabels] = useState<RefObject<HTMLSpanElement>[]>(
[]
)
const isRangeSlider = Array.isArray(value)

useEffect(() => {
if (!isRangeSlider) {
if (!isRangeSlider || !isTooltipRendered) {
return
}
const isFullyOverlaped = value[0] === value[1]
Expand All @@ -31,7 +37,13 @@ export const useLabelOverlap = ({ value }: { value?: number | number[] }) => {
})
)
}
}, [value, isRangeSlider, isPartiallyOverlapped, valueLabels])
}, [
value,
isRangeSlider,
isPartiallyOverlapped,
valueLabels,
isTooltipRendered,
])

const handleValueLabelOnRender = useCallback(
(index: number, labelRef: RefObject<HTMLSpanElement>) => {
Expand Down
10 changes: 6 additions & 4 deletions packages/base/Slider/src/Slider/story/Default.example.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
import React from 'react'
import React, { useState } from 'react'
import { Container, Slider } from '@toptal/picasso'

type Value = number | number[]

const Example = () => {
const handleChange = (_: React.ChangeEvent<{}>, value: Value) => {
window.console.log('onChange: ', value)
const [value, setValue] = useState<Value>(0)

const handleChange = (_: Event, newValue: Value) => {
setValue(newValue)
}

return (
<Container>
<Slider onChange={handleChange} />
<Slider value={value} onChange={handleChange} />
</Container>
)
}
Expand Down
14 changes: 4 additions & 10 deletions packages/base/Slider/src/Slider/test.tsx
Original file line number Diff line number Diff line change
@@ -1,29 +1,23 @@
import type { ReactNode } from 'react'
import { describe, expect, it } from '@jest/globals'
import React from 'react'
import type { RenderResult } from '@testing-library/react'
import { render } from '@testing-library/react'
import type { OmitInternalProps } from '@toptal/picasso-shared'

jest.mock('@toptal/picasso-utils', () => ({
...jest.requireActual('@toptal/picasso-utils'),
useOnScreen: jest.fn().mockRejectedValueOnce(true),
}))

import type { Props } from './Slider'
import { Slider } from './Slider'

const renderSlider = (children: ReactNode, props: OmitInternalProps<Props>) => {
const renderSlider = (props: OmitInternalProps<Props>) => {
const { value } = props

return render(<Slider value={value}>{children}</Slider>)
return render(<Slider value={value} />)
}

describe('Slider', () => {
let api: RenderResult

beforeEach(() => {
api = renderSlider(null, {})
api = renderSlider({})
})

it('renders', () => {
Expand All @@ -33,7 +27,7 @@ describe('Slider', () => {
})

it('with initial value', () => {
const { container } = renderSlider(null, { value: 4 })
const { container } = renderSlider({ value: 4 })

expect(container).toMatchSnapshot()
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ const SliderValueLabel = ({
})
)
// we need to recalculate on value change to get new rect
}, [isOverlaped, index, xPlacement, value])
}, [isOverlaped, index, xPlacement, value, tooltip])

return (
<span
Expand Down
8 changes: 5 additions & 3 deletions packages/base/Utils/src/utils/useOnScreen/use-on-screen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@ const useOnScreen = ({
rootMargin,
threshold,
}: UseOnScreenProps) => {
const [isIntersecting, setIntersecting] = useState(false)
const [isOnScreen, setIsOnScreen] = useState(false)
const [isObserved, setObserved] = useState(false)

const observer = useMemo(
() =>
new IntersectionObserver(
([entry]) => {
setIntersecting(entry.isIntersecting)
setIsOnScreen(entry.isIntersecting)
setObserved(true)
},
{
root: root?.current,
Expand All @@ -44,7 +46,7 @@ const useOnScreen = ({
}
}, [observer, ref])

return isIntersecting
return { isOnScreen, isObserved }
}

export default useOnScreen
5 changes: 4 additions & 1 deletion tailwind.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@

/** @type {import('tailwindcss').Config} */
module.exports = {
content: ['packages/**/*.{ts,tsx}'],
content: [
'packages/base/*/src/**/*.{ts,tsx}',
'packages/*/src/**/*.{ts,tsx}',
],
presets: [
require('@toptal/base-tailwind'),
require('@toptal/picasso-tailwind'),
Expand Down

0 comments on commit 0dbab90

Please sign in to comment.