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

fix(react-color-picker): place thumb outside of Sliders and ColorArea #33880

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

🕵🏾‍♀️ visual regressions to review in the fluentuiv9 Visual Regression Report

Avatar Converged 1 screenshots
Image Name Diff(in Pixels) Image Type
Avatar Converged.badgeMask.normal.chromium.png 2 Changed
ColorPicker Converged 6 screenshots
Image Name Diff(in Pixels) Image Type
ColorPicker Converged.Default.default.chromium.png 205 Changed
ColorPicker Converged.Default - High Contrast.default.chromium.png 301 Changed
ColorPicker Converged.Alpha Sliders.default.chromium.png 66 Changed
ColorPicker Converged.Default - RTL.default.chromium.png 205 Changed
ColorPicker Converged.Default - Dark Mode.default.chromium.png 195 Changed
ColorPicker Converged.shape.default.chromium.png 213 Changed

"type": "patch",
"comment": "fix: placed thumb half outside of colorSlider and colorArea",
"packageName": "@fluentui/react-color-picker-preview",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,10 @@ export const alphaSliderCSSVars = {
railColorVar: `--fui-AlphaSlider__rail--color`,
};

// Internal CSS variables
const thumbPositionVar = `--fui-AlphaSlider__thumb--position`;
const innerThumbRadiusVar = `--fui-AlphaSlider__thumb--radius`;

/**
* Styles for the root slot
*/
const useStyles = makeStyles({
root: {
[innerThumbRadiusVar]: '6px',
},
rail: {
border: `1px solid ${tokens.colorNeutralStroke1}`,
backgroundImage: `linear-gradient(var(${alphaSliderCSSVars.sliderDirectionVar}), transparent, var(${alphaSliderCSSVars.railColorVar})), url(${TRANSPARENT_IMAGE_URL})`,
Expand All @@ -43,18 +36,17 @@ const useStyles = makeStyles({
const useThumbStyles = makeStyles({
thumb: {
backgroundColor: tokens.colorNeutralBackground1,
[`${thumbPositionVar}`]: `clamp(var(${innerThumbRadiusVar}), var(${alphaSliderCSSVars.sliderProgressVar}), calc(100% - var(${innerThumbRadiusVar})))`,
'::before': {
backgroundColor: `var(${alphaSliderCSSVars.thumbColorVar})`,
},
},
horizontal: {
transform: 'translateX(-50%)',
left: `var(${thumbPositionVar})`,
left: `var(${alphaSliderCSSVars.sliderProgressVar})`,
},
vertical: {
transform: 'translateY(50%)',
bottom: `var(${thumbPositionVar})`,
bottom: `var(${alphaSliderCSSVars.sliderProgressVar})`,
},
});

Expand All @@ -66,7 +58,7 @@ export const useAlphaSliderStyles_unstable = (state: AlphaSliderState): AlphaSli

const styles = useStyles();
const thumbStyles = useThumbStyles();
state.root.className = mergeClasses(alphaSliderClassNames.root, styles.root, state.root.className);
state.root.className = mergeClasses(alphaSliderClassNames.root, state.root.className);
state.input.className = mergeClasses(alphaSliderClassNames.input, state.input.className);
state.rail.className = mergeClasses(alphaSliderClassNames.rail, styles.rail, state.rail.className);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@ export const colorAreaCSSVars = {

// Internal CSS variables
const thumbSizeVar = `--fui-Slider__thumb--size`;
const thumbPositionXVar = `--fui-AlphaSlider__thumb--positionX`;
const thumbPositionYVar = `--fui-AlphaSlider__thumb--positionY`;
const innerThumbRadiusVar = `--fui-AlphaSlider__thumb--radius`;

/**
* Styles for the root slot
Expand All @@ -40,7 +37,6 @@ const useRootStyles = makeResetStyles({
minWidth: '300px',
minHeight: '300px',
boxSizing: 'border-box',
[innerThumbRadiusVar]: '6px',
});

/**
Expand All @@ -59,10 +55,8 @@ const useThumbStyles = makeStyles({
boxShadow: tokens.shadow4,
backgroundColor: `var(${colorAreaCSSVars.thumbColorVar})`,
transform: 'translate(-50%, 50%)',
[`${thumbPositionXVar}`]: `clamp(var(${innerThumbRadiusVar}), var(${colorAreaCSSVars.areaXProgressVar}), calc(100% - var(${innerThumbRadiusVar})))`,
[`${thumbPositionYVar}`]: `clamp(var(${innerThumbRadiusVar}), var(${colorAreaCSSVars.areaYProgressVar}), calc(100% - var(${innerThumbRadiusVar})))`,
left: `var(${thumbPositionXVar})`,
bottom: `var(${thumbPositionYVar})`,
left: `var(${colorAreaCSSVars.areaXProgressVar})`,
bottom: `var(${colorAreaCSSVars.areaYProgressVar})`,
'::before': {
position: 'absolute',
inset: '0px',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { makeStyles, mergeClasses } from '@griffel/react';
import type { SlotClassNames } from '@fluentui/react-utilities';
import type { ColorPickerSlots, ColorPickerState } from './ColorPicker.types';

import { THUMB_SIZE } from '../../utils/constants';
export const colorPickerClassNames: SlotClassNames<ColorPickerSlots> = {
root: 'fui-ColorPicker',
};
Expand All @@ -13,6 +13,7 @@ const useStyles = makeStyles({
root: {
display: 'flex',
flexDirection: 'column',
gap: `${THUMB_SIZE / 2}px`,
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { makeStyles, makeResetStyles, mergeClasses } from '@griffel/react';
import type { SlotClassNames } from '@fluentui/react-utilities';
import { tokens } from '@fluentui/react-theme';
import type { ColorSliderSlots, ColorSliderState } from './ColorSlider.types';
import { THUMB_SIZE } from '../../utils/constants';

export const colorSliderClassNames: SlotClassNames<ColorSliderSlots> = {
root: 'fui-ColorSlider',
Expand All @@ -15,14 +16,10 @@ export const colorSliderCSSVars = {
sliderProgressVar: `--fui-Slider--progress`,
thumbColorVar: `--fui-Slider__thumb--color`,
railColorVar: `--fui-Slider__rail--color`,
thumbSizeVar: `--fui-Slider__thumb--size`,
railSizeVar: `--fui-Slider__rail--size`,
};

// Internal CSS variables
const thumbSizeVar = `--fui-Slider__thumb--size`;
const railSizeVar = `--fui-Slider__rail--size`;
const innerThumbRadiusVar = `--fui-Slider__inner-thumb--radius`;
const thumbPositionVar = `--fui-Slider__thumb--position`;

const hueBackground = `linear-gradient(${[
`var(${colorSliderCSSVars.sliderDirectionVar})`,
'red',
Expand All @@ -43,25 +40,24 @@ const useRootStyles = makeResetStyles({
touchAction: 'none',
alignItems: 'center',
justifyItems: 'center',
[thumbSizeVar]: '20px',
[railSizeVar]: '20px',
[innerThumbRadiusVar]: '6px',
[colorSliderCSSVars.thumbSizeVar]: `${THUMB_SIZE}px`,
[colorSliderCSSVars.railSizeVar]: `${THUMB_SIZE}px`,
minHeight: '32px',
});

const useStyles = makeStyles({
horizontal: {
minWidth: '200px',
// 3x3 grid with the rail and thumb in the center cell [2,2] and the hidden input stretching across all cells
gridTemplateRows: `1fr var(${thumbSizeVar}) 1fr`,
gridTemplateRows: `1fr var(${colorSliderCSSVars.thumbSizeVar}) 1fr`,
gridTemplateColumns: `1fr 100% 1fr`,
},

vertical: {
minHeight: '280px',
// 3x3 grid with the rail and thumb in the center cell [2,2] and the hidden input stretching across all cells
gridTemplateRows: `1fr 100% 1fr`,
gridTemplateColumns: `1fr var(${thumbSizeVar}) 1fr`,
gridTemplateColumns: `1fr var(${colorSliderCSSVars.thumbSizeVar}) 1fr`,
},
});

Expand Down Expand Up @@ -100,19 +96,19 @@ const useRailStyles = makeStyles({

horizontal: {
width: '100%',
height: `var(${railSizeVar})`,
height: `var(${colorSliderCSSVars.railSizeVar})`,
'::before': {
left: '-1px',
right: '-1px',
height: `var(${railSizeVar})`,
height: `var(${colorSliderCSSVars.railSizeVar})`,
},
},

vertical: {
width: `var(${railSizeVar})`,
width: `var(${colorSliderCSSVars.railSizeVar})`,
height: '100%',
'::before': {
width: `var(${railSizeVar})`,
width: `var(${colorSliderCSSVars.railSizeVar})`,
top: '-1px',
bottom: '1px',
},
Expand All @@ -129,16 +125,15 @@ const useThumbStyles = makeStyles({
gridColumnStart: '2',
gridColumnEnd: '2',
position: 'absolute',
width: `var(${thumbSizeVar})`,
height: `var(${thumbSizeVar})`,
width: `var(${colorSliderCSSVars.thumbSizeVar})`,
height: `var(${colorSliderCSSVars.thumbSizeVar})`,
pointerEvents: 'none',
outlineStyle: 'none',
forcedColorAdjust: 'none',
borderRadius: tokens.borderRadiusCircular,
border: `${tokens.strokeWidthThin} solid ${tokens.colorNeutralForeground4}`,
boxShadow: tokens.shadow4,
backgroundColor: `var(${colorSliderCSSVars.thumbColorVar})`,
[`${thumbPositionVar}`]: `clamp(var(${innerThumbRadiusVar}), var(${colorSliderCSSVars.sliderProgressVar}), calc(100% - var(${innerThumbRadiusVar})))`,
'::before': {
position: 'absolute',
inset: '0px',
Expand All @@ -150,11 +145,11 @@ const useThumbStyles = makeStyles({
},
horizontal: {
transform: 'translateX(-50%)',
left: `var(${thumbPositionVar})`,
left: `var(${colorSliderCSSVars.sliderProgressVar})`,
},
vertical: {
transform: 'translateY(50%)',
bottom: `var(${thumbPositionVar})`,
bottom: `var(${colorSliderCSSVars.sliderProgressVar})`,
},
});

Expand Down Expand Up @@ -187,12 +182,12 @@ const useInputStyles = makeStyles({
},
},
horizontal: {
height: `var(${thumbSizeVar})`,
height: `var(${colorSliderCSSVars.thumbSizeVar})`,
width: '100%',
},
vertical: {
height: '100%',
width: `var(${thumbSizeVar})`,
width: `var(${colorSliderCSSVars.thumbSizeVar})`,
'writing-mode': 'vertical-lr',
direction: 'rtl',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@ export const MAX = 100;
export const HUE_MAX = 360;
export const INITIAL_COLOR = '#FFF';
export const INITIAL_COLOR_HSV = tinycolor(INITIAL_COLOR).toHsv();

export const THUMB_SIZE = 20;