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.

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

feat(react-swatch-picker): Added borderColor prop and fixed styles #31358

Conversation

ValentinaKozlova
Copy link
Contributor

@ValentinaKozlova ValentinaKozlova commented May 14, 2024

New Behavior

  • Added contrast utils and tests
  • fixed positioning of icon/initials in disabled state
  • added shadow for disabled icon
  • Added example with low contrast colors

image

@ValentinaKozlova ValentinaKozlova force-pushed the feat/swatch-picker-contrast-ratio branch from 3674ecb to 46e3bfc Compare May 14, 2024 07:25
@ValentinaKozlova ValentinaKozlova marked this pull request as ready for review May 14, 2024 07:25
@ValentinaKozlova ValentinaKozlova requested a review from a team as a code owner May 14, 2024 07:25
@fabricteam
Copy link
Collaborator

fabricteam commented May 14, 2024

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-swatch-picker-preview
@fluentui/react-swatch-picker-preview - package
109.013 kB
30.362 kB
109.288 kB
30.485 kB
275 B
123 B

🤖 This report was generated against 79b73d920ef1a89e5daff165f28f0afe966afb01

Copy link

codesandbox-ci bot commented May 14, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Copy link
Member

@layershifter layershifter left a comment

Choose a reason for hiding this comment

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

  • Let's ensure that we have the best possible behavior built-in, at least for ColorSwatch it seems to be achievable
  • We need to have color computations under useEffect (probably useLayoutEffect() to avoid flickering)
    • I would propose something like that (we will avoid state updates i.e. re-renders 👍 ):
    const { color } = props
    const ref = React.useRef();
    
    React.useLayoutEffect(() => {
      const constrastColor = DO_SOMETHING(color);
    
      ref.current.style.setProperty('--fui-switch-SMTH', constrastColor);
    }, [color])
  • We need to handle scenarios with CSS variables
  • We need more powerful color parsing to handle cases not only for rgb() or HEX values. color-parse looks promising (5kb minified)

@ValentinaKozlova
Copy link
Contributor Author

  • Let's ensure that we have the best possible behavior built-in, at least for ColorSwatch it seems to be achievable

  • We need to have color computations under useEffect (probably useLayoutEffect() to avoid flickering)

    • I would propose something like that (we will avoid state updates i.e. re-renders 👍 ):
    const { color } = props
    const ref = React.useRef();
    
    React.useLayoutEffect(() => {
      const constrastColor = DO_SOMETHING(color);
    
      ref.current.style.setProperty('--fui-switch-SMTH', constrastColor);
    }, [color])
  • We need to handle scenarios with CSS variables

  • We need more powerful color parsing to handle cases not only for rgb() or HEX values. color-parse looks promising (5kb minified)

Yeah, it's a good idea. We'll add those after going stable

@ValentinaKozlova ValentinaKozlova merged commit 1cc1aa3 into microsoft:master May 15, 2024
19 checks passed
@ValentinaKozlova ValentinaKozlova deleted the feat/swatch-picker-contrast-ratio branch May 15, 2024 15:04
@ValentinaKozlova ValentinaKozlova changed the title feat(react-swatch-picker): Added contrast ratio utils feat(react-swatch-picker): Added borderColor prop and fixed styles May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants