-
Notifications
You must be signed in to change notification settings - Fork 7
feat(design-system): create reusable helper function to support responsive styling [AR-53842] #338
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
3835a39
ef9b463
c1bcad4
633a1fd
9d386ca
bcb5b77
644fae5
a345b24
f109f58
569cfcb
77cd41e
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,5 @@ | ||
| --- | ||
| '@drivenets/design-system': minor | ||
| --- | ||
|
|
||
| Add responsive prop support at 1440px breakpoint with CSS-first styling and `useResponsiveValue` hook for JS conditional rendering |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,5 @@ | ||
| export { default as DsButtonNew } from './ds-button-new'; | ||
| import { withResponsiveProps } from '../../../../utils/responsive'; | ||
| import DsButtonBase from './ds-button-new'; | ||
|
|
||
| export const DsButtonNew = withResponsiveProps(DsButtonBase, ['size']); | ||
| export * from './ds-button-new.types'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| @use 'variables' as var; | ||
|
|
||
| :export { | ||
| breakpointLg: #{var.$breakpoint-lg}; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| @use 'variables' as var; | ||
|
|
||
| @mixin lg { | ||
| @media (min-width: #{var.$breakpoint-lg}) { | ||
| @content; | ||
| } | ||
| } | ||
|
|
||
| @mixin md { | ||
| @media (max-width: #{var.$breakpoint-lg - 1}) { | ||
| @content; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| import { createElement } from 'react'; | ||
| import { describe, expect, it, vi } from 'vitest'; | ||
| import { page } from 'vitest/browser'; | ||
| import { renderHook } from 'vitest-browser-react'; | ||
|
|
||
| import { useBreakpoint, useResponsiveValue, withResponsiveProps } from './responsive'; | ||
|
|
||
| const mockMatchMedia = (matches: boolean) => { | ||
| const listeners: Array<() => void> = []; | ||
|
|
||
| Object.defineProperty(window, 'matchMedia', { | ||
| writable: true, | ||
| value: vi.fn().mockReturnValue({ | ||
| matches, | ||
| addEventListener: (_: string, cb: () => void) => listeners.push(cb), | ||
| removeEventListener: (_: string, cb: () => void) => { | ||
| const idx = listeners.indexOf(cb); | ||
|
|
||
| if (idx >= 0) { | ||
| listeners.splice(idx, 1); | ||
| } | ||
| }, | ||
| }), | ||
| }); | ||
|
|
||
| return { | ||
| triggerChange: () => listeners.forEach((cb) => cb()), | ||
| }; | ||
| }; | ||
|
|
||
| describe('useBreakpoint', () => { | ||
| it('should return lg when viewport >= 1440px', async () => { | ||
| mockMatchMedia(true); | ||
| const { result } = await renderHook(() => useBreakpoint()); | ||
|
|
||
| expect(result.current).toBe('lg'); | ||
| }); | ||
|
|
||
| it('should return md when viewport < 1440px', async () => { | ||
| mockMatchMedia(false); | ||
| const { result } = await renderHook(() => useBreakpoint()); | ||
|
|
||
| expect(result.current).toBe('md'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('useResponsiveValue', () => { | ||
| it('should return a static value unchanged', async () => { | ||
| mockMatchMedia(true); | ||
| const { result } = await renderHook(() => useResponsiveValue('large')); | ||
|
|
||
| expect(result.current).toBe('large'); | ||
| }); | ||
|
|
||
| it('should resolve lg value on large screens', async () => { | ||
| mockMatchMedia(true); | ||
| const { result } = await renderHook(() => useResponsiveValue({ lg: 'large', md: 'small' })); | ||
|
|
||
| expect(result.current).toBe('large'); | ||
| }); | ||
|
|
||
| it('should resolve md value on small screens', async () => { | ||
| mockMatchMedia(false); | ||
| const { result } = await renderHook(() => useResponsiveValue({ lg: 'large', md: 'small' })); | ||
|
|
||
| expect(result.current).toBe('small'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('withResponsiveProps', () => { | ||
| const Base = ({ value }: { value?: string }) => createElement('span', null, value); | ||
| const Enhanced = withResponsiveProps(Base, ['value']); | ||
|
|
||
| it('should resolve responsive prop to lg value on large screens', async () => { | ||
| mockMatchMedia(true); | ||
| await page.render(createElement(Enhanced, { value: { lg: 'desktop', md: 'mobile' } })); | ||
|
|
||
| await expect.element(page.getByText('desktop')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('should resolve responsive prop to md value on small screens', async () => { | ||
| mockMatchMedia(false); | ||
| await page.render(createElement(Enhanced, { value: { lg: 'desktop', md: 'mobile' } })); | ||
|
|
||
| await expect.element(page.getByText('mobile')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('should pass through static values unchanged', async () => { | ||
| mockMatchMedia(true); | ||
| await page.render(createElement(Enhanced, { value: 'static' })); | ||
|
|
||
| await expect.element(page.getByText('static')).toBeInTheDocument(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| import { createElement, useSyncExternalStore, type FunctionComponent } from 'react'; | ||
|
|
||
| import breakpointTokens from '../styles/_breakpoints.module.scss'; | ||
|
|
||
| export const BREAKPOINT_LG = parseInt(breakpointTokens.breakpointLg, 10); | ||
|
|
||
| export const breakpoints = ['lg', 'md'] as const; | ||
| export type Breakpoint = (typeof breakpoints)[number]; | ||
|
|
||
| export type Responsive<T> = Partial<Record<Breakpoint, T>>; | ||
|
|
||
| export type ResponsiveValue<T> = T | Responsive<T>; | ||
|
|
||
| export const isResponsiveValue = <T>(value: ResponsiveValue<T>): value is Responsive<T> => | ||
| value !== null && typeof value === 'object' && breakpoints.some((bp) => bp in value); | ||
|
|
||
| export const resolveResponsiveValue = <T>(value: ResponsiveValue<T>, breakpoint: Breakpoint): T => { | ||
| if (!isResponsiveValue(value)) { | ||
| return value; | ||
| } | ||
|
|
||
| return (value[breakpoint] ?? value.lg ?? value.md) as T; | ||
| }; | ||
|
|
||
| const MEDIA_QUERY = `(min-width: ${String(BREAKPOINT_LG)}px)`; | ||
|
|
||
| const subscribe = (callback: () => void) => { | ||
| const mql = window.matchMedia(MEDIA_QUERY); | ||
| mql.addEventListener('change', callback); | ||
| return () => mql.removeEventListener('change', callback); | ||
| }; | ||
|
|
||
| const getSnapshot = (): Breakpoint => (window.matchMedia(MEDIA_QUERY).matches ? 'lg' : 'md'); | ||
|
|
||
| const getServerSnapshot = (): Breakpoint => 'lg'; | ||
|
Collaborator
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. It returns static value, why is that? Assumption server will always render the 'lg', right?
Collaborator
Author
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. As we do not have info during SSR about screen size it is fallback for lg size |
||
|
|
||
| export const useBreakpoint = (): Breakpoint => | ||
| useSyncExternalStore(subscribe, getSnapshot, getServerSnapshot); | ||
|
|
||
| /** | ||
| * Resolves a `ResponsiveValue<T>` to `T` using the current breakpoint. | ||
| * Use for conditional rendering or JS logic that depends on the breakpoint. | ||
| */ | ||
| export const useResponsiveValue = <T>(value: ResponsiveValue<T>): T => | ||
| resolveResponsiveValue(value, useBreakpoint()); | ||
|
|
||
| /** | ||
| * Wraps a component so that the specified props accept `ResponsiveValue<T>`. | ||
| * The wrapper resolves each responsive prop to a plain value before rendering, | ||
| * so the wrapped component stays completely unaware of breakpoints. | ||
| */ | ||
| export function withResponsiveProps<Props, const Keys extends readonly (keyof Props)[]>( | ||
| Component: FunctionComponent<Props>, | ||
| responsiveKeys: Keys, | ||
| ) { | ||
| type EnhancedProps = { | ||
| [P in keyof Props]: P extends Keys[number] ? ResponsiveValue<NonNullable<Props[P]>> : Props[P]; | ||
| }; | ||
|
|
||
| const Wrapper = (props: EnhancedProps) => { | ||
| const breakpoint = useBreakpoint(); | ||
| const resolved = { ...props } as Record<string, unknown>; | ||
|
Collaborator
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. There is a lot of type casting. Can you verify whether it can be avoided?
Collaborator
Author
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.
removed cast for string |
||
|
|
||
| for (const key of responsiveKeys) { | ||
| const k = String(key); | ||
| const value = resolved[k]; | ||
|
|
||
| if (value !== undefined) { | ||
| resolved[k] = resolveResponsiveValue(value, breakpoint); | ||
| } | ||
| } | ||
|
|
||
| return createElement(Component as FunctionComponent<Record<string, unknown>>, resolved); | ||
| }; | ||
|
|
||
| Wrapper.displayName = `withResponsiveProps(${Component.displayName ?? Component.name})`; | ||
|
|
||
| return Wrapper; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
|
|
||
| import { BREAKPOINT_LG, isResponsiveValue, resolveResponsiveValue } from './responsive'; | ||
|
|
||
| describe('BREAKPOINT_LG', () => { | ||
| it('should be sourced from the SCSS variable and equal 1440', () => { | ||
| expect(BREAKPOINT_LG).toBe(1440); | ||
| }); | ||
| }); | ||
|
|
||
| describe('isResponsiveValue', () => { | ||
| it('should return true for a responsive object', () => { | ||
| expect(isResponsiveValue({ lg: 'large', md: 'small' })).toBe(true); | ||
| }); | ||
|
|
||
| it('should return false for a string', () => { | ||
| expect(isResponsiveValue('large')).toBe(false); | ||
| }); | ||
|
|
||
| it('should return false for null', () => { | ||
| expect(isResponsiveValue(null as unknown as string)).toBe(false); | ||
| }); | ||
|
|
||
| it('should return true for an object with only lg', () => { | ||
| expect(isResponsiveValue({ lg: 'large' })).toBe(true); | ||
| }); | ||
|
|
||
| it('should return true for an object with only md', () => { | ||
| expect(isResponsiveValue({ md: 'small' })).toBe(true); | ||
| }); | ||
| }); | ||
|
|
||
| describe('resolveResponsiveValue', () => { | ||
| it('should return static value unchanged for both breakpoints', () => { | ||
| expect(resolveResponsiveValue('large', 'lg')).toBe('large'); | ||
| expect(resolveResponsiveValue('large', 'md')).toBe('large'); | ||
| }); | ||
|
|
||
| it('should resolve lg value from responsive object', () => { | ||
| expect(resolveResponsiveValue({ lg: 'large', md: 'small' }, 'lg')).toBe('large'); | ||
| }); | ||
|
|
||
| it('should resolve md value from responsive object', () => { | ||
| expect(resolveResponsiveValue({ lg: 'large', md: 'small' }, 'md')).toBe('small'); | ||
| }); | ||
|
|
||
| it('should fall back to lg when md is not specified', () => { | ||
| expect(resolveResponsiveValue({ lg: 'large' }, 'md')).toBe('large'); | ||
| }); | ||
|
|
||
| it('should fall back to md when lg is not specified', () => { | ||
| expect(resolveResponsiveValue({ md: 'small' }, 'lg')).toBe('small'); | ||
| }); | ||
| }); |
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.
Pick question: Does it work in the build script? Just to be sure it's picked up, maybe something compiler/bundler related could be missing
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.
working well