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

RangeControl: Update the default marks styles to match the padding/margin control #67611

Merged
merged 3 commits into from
Dec 6, 2024
Merged

RangeControl: Update the default marks styles to match the padding/margin control #67611

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
28b5eb0
RangeControl: Update the default marks styles to match the padding/ma…
youknowriad Dec 5, 2024
670a43e
Add changelog note
youknowriad Dec 5, 2024
25679e2
Code style tweaks
youknowriad Dec 5, 2024
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
29 changes: 0 additions & 29 deletions packages/block-editor/src/components/spacing-sizes-control/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,40 +4,11 @@
margin-bottom: 0;
}

.is-marked {
.components-range-control__track {
transition: width ease 0.1s;
@include reduce-motion("transition");
}

.components-range-control__thumb-wrapper {
transition: left ease 0.1s;
@include reduce-motion("transition");
}
}

.spacing-sizes-control__range-control,
.spacing-sizes-control__custom-value-range {
flex: 1;
margin-bottom: 0; // Needed for some instances of the range control, such as the Spacer block.
}

.components-range-control__mark {
transform: translateX(-50%);
height: $grid-unit-05;
width: math.div($grid-unit-05, 2);
background-color: $white;
z-index: 1;
top: -#{$grid-unit-05};
}

.components-range-control__marks {
margin-top: 17px;
}

.components-range-control__thumb-wrapper {
z-index: 3;
}
}

.spacing-sizes-control__header {
Expand Down
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

- `GradientPicker`: Add `enableAlpha` prop ([#66974](https://github.com/WordPress/gutenberg/pull/66974))
- `CustomGradientPicker`: Add `enableAlpha` prop ([#66974](https://github.com/WordPress/gutenberg/pull/66974))
- `RangeControl`: Update the design of the range control marks ([#67611](https://github.com/WordPress/gutenberg/pull/67611))

### Deprecations

Expand Down
1 change: 0 additions & 1 deletion packages/components/src/range-control/mark.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ export default function RangeMark(
{ ...otherProps }
aria-hidden="true"
className={ classes }
isFilled={ isFilled }
style={ style }
/>
{ label && (
Expand Down
35 changes: 17 additions & 18 deletions packages/components/src/range-control/styles/range-control-styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,10 @@ export const Track = styled.span`
margin-top: ${ ( rangeHeightValue - railHeight ) / 2 }px;
top: 0;

@media not ( prefers-reduced-motion ) {
transition: width ease 0.1s;
}
Comment on lines +133 to +135
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this animation is there to go hand in hand with the thumb's animation — we should remove it if we remove the thumb's animation.

(sidenote: animating the width is not very performant, and can cause the UI to feel sluggish instead of polishing the animation).

If were to keep the animation around, we could add a comment to help future readers understand that the track and thumb animations go hand in hand.


${ trackBackgroundColor };
`;

Expand All @@ -139,28 +143,18 @@ export const MarksWrapper = styled.span`
position: relative;
width: 100%;
user-select: none;
margin-top: 17px;
`;

const markFill = ( { disabled, isFilled }: RangeMarkProps ) => {
let backgroundColor = isFilled ? 'currentColor' : COLORS.gray[ 300 ];

if ( disabled ) {
backgroundColor = COLORS.gray[ 400 ];
}

return css( {
backgroundColor,
} );
};

export const Mark = styled.span`
height: ${ thumbSize }px;
left: 0;
position: absolute;
top: 9px;
width: 1px;

${ markFill };
left: 0;
top: -4px;
height: 4px;
width: 2px;
transform: translateX( -50% );
background-color: ${ COLORS.white };
youknowriad marked this conversation as resolved.
Show resolved Hide resolved
z-index: 1;
`;

const markLabelFill = ( { isFilled }: RangeMarkProps ) => {
Expand Down Expand Up @@ -207,6 +201,11 @@ export const ThumbWrapper = styled.span`
user-select: none;
width: ${ thumbSize }px;
border-radius: ${ CONFIG.radiusRound };
z-index: 3;

@media not ( prefers-reduced-motion ) {
transition: left ease 0.1s;
}
Comment on lines +206 to +208
Copy link
Contributor

Choose a reason for hiding this comment

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

This animation introduces a subtle delay when dragging the thumb that can make the UI feel unresponsive. We could consider removing it for the time being, and focus this PR on updating the marks design.

(also, in the block editor "custom" version, this animation was only applied when the is-marked class is applied — just noting in case that was deliberate)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The animation already exists in padding/margin so removing it could be seen as a regression in the behavior of the component.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jasmussen should we remove the animation or keep it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's the animation, though in low framerate GIF:

with animation

Note the slight disconnect with the cursor.

Here's with no animation:

no animation

It's not a huge difference, though I tend to think with animation is preferred, as it make it "snap" into place a bit more.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it — I will then propose that we keep the animation only when the component is showing marks (like it was in the deleted code from packages/block-editor/src/components/spacing-sizes-control/style.scss). I can open a PR for that


${ thumbColor };
${ rtl( { marginLeft: -10 } ) };
Expand Down
Loading