`AnglePickerControl`: Style to better fit in narrow contexts and improve RTL layout #49046

stokesman · 2023-03-13T17:59:09Z

What?

Makes the more room for the number input of the control
For RTL, puts the degree symbol to the right of the number input
A bit of refactoring to simplify styles

Why?

Will close GradientPicker value gets cut off in block editor sidebar Popovers #45781
Will close AnglePickerControl: RTL layout of degree symbol #35710

How?

Reduces the gap between its constituent parts
Conditionally uses the prefix prop for the angle symbol when RTL

Testing Instructions

In either the Post Editor or Global Styles edit/apply a gradient
Verify that three digit numbers are not cut off
Verify that no other visual changes are introduced

Testing Instructions for Keyboard

Nothing has really changed for pure keyboard use. The knob is not tabbable in either trunk or this branch.

In either the Post Editor or Global Styles edit/apply a gradient
Tab into the “Angle” field and use the arrow keys to adjust the value

Screenshots or screencast

Before	After

RTL Before	RTL After

Archived from earlier iterations

In Post Editor adjusting a gradient

angle-picker-control-knob-affixed.mp4

Note the easy keyboard adjustment after dragging since the number input is already focused.

In Global Styles

angle-picker-knob-affixed-global-styles-sidebar

Other screenshots to show design alternatives

A before/after of just moving the knob “inside” without other style changes

A style experiment I tried along the way

angle-picker-control-try-alt-style.mp4

jasmussen · 2023-03-14T09:50:21Z

Thanks for the PR!

I'm not sure it's good to change the morphology of the angle picker quite yet. I don't hate the line, but I also don't think it's necessarily clearer that it points in a particular direction. I think there's an opportunity to refine the design, to be clear, but it might be best to keep that separate from the other excellent bugfixes. What do you think?

stokesman · 2023-03-14T18:07:05Z

I don't have a strong opinion on the line versus the dot. Here’s a before/after comparison of just moving the knob inside:

Would that be approvable? I'd still want to reduce the apparent size but it's not necessary.

Also, for a bit of context, the idea to try this came from the recent custom spinner controls:

jasmussen · 2023-03-15T07:53:08Z

A simpler change might be to change the gap value from 16px to 8px.

Before:

After:

What do you think?

stokesman · 2023-03-16T06:43:46Z

What do you think?

It's great! Also it's done. I've updated the PR description and screenshots. It's funny how that gap has bugged me for quite some time. I just got a little tunnel vision on that other idea. Thanks for getting your hands dirty there Joen!

jasmussen

Looks good to me. Before, cropped text in the angle field:

After, uncropped:

I'm mainly reviewing this visually, so I'd appreciate a sanity check from a dev (maybe cc @mirka?). But otherwise, looks good, thanks for the PR!

packages/components/src/angle-picker-control/angle-circle.tsx

jasmussen · 2023-03-16T09:03:06Z

packages/components/src/angle-picker-control/index.tsx

 	return (
 		<Root
 			{ ...restProps }
 			ref={ ref }
 			__nextHasNoMarginBottom={ __nextHasNoMarginBottom }
 			className={ classes }
-			gap={ 4 }
+			gap={ 2 }


jasmussen · 2023-03-16T09:03:30Z

packages/components/src/angle-picker-control/styles/angle-picker-control-styles.tsx

 import CONFIG from '../../utils/config-values';

 import type { AnglePickerControlProps } from '../types';

 const CIRCLE_SIZE = 32;
-const INNER_CIRCLE_SIZE = 3;
+const INNER_CIRCLE_SIZE = 6;


What does this one actually do? Mostly for curiosity.

It's the size of the dot on the knob. Previously it was used as a radius. As part of simplifying the styles used to draw the dot it was more straightforward to use as a diameter (which also makes it consistent with CIRCLE_SIZE which is a diameter).

Oh interesting, thanks for the response! I don't know that we'd ever want to actually change the size of the knob situationally, so I guess it good that it isn't surfaced as a prop :)

jasmussen · 2023-03-16T09:03:39Z

packages/components/src/angle-picker-control/styles/angle-picker-control-styles.tsx

@@ -39,31 +40,33 @@ export const CircleRoot = styled.div`
 	height: ${ CIRCLE_SIZE }px;
 	overflow: hidden;
 	width: ${ CIRCLE_SIZE }px;
+
+	:active {
+		cursor: grabbing;


mirka

Very nice fixes, thank you! Let us know when this is ready for a full code review 🙂 It might be worth splitting this into at least two PRs since it's touching several independent concerns. Would be easier to pin down if there are any regressions.

packages/components/src/angle-picker-control/index.tsx

stokesman · 2023-03-17T05:28:48Z

Thanks for testing and reviewing Joen!

I've taken Lena’s advice and dropped any changes here besides the visual stuff. So I'll go ahead and mark this as ready. Oh that and add to the changelog.

github-actions · 2023-03-17T05:36:26Z

Flaky tests detected in 6835eea.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4450374242
📝 Reported issues:

[Flaky Test] allows shift tabbing to the block toolbar from the first block #49025 in /test/e2e/specs/site-editor/writing-flow.spec.js

mirka

Looks great, thanks for the polish! 🚀

mirka · 2023-03-17T16:27:17Z

packages/components/src/angle-picker-control/index.tsx

-					marginTop: 'auto',
-				} }
-			>
+			<Spacer marginBottom="1" marginTop="auto">


Interesting. Not for this PR, but for layouts like this I'd probably first try using the NumberControl without a label, and wrap the entire thing in a separate BaseControl. I hope our components support that kind of composability without messing up the HTML semantics.

mirka · 2023-03-17T16:33:57Z

packages/components/CHANGELOG.md

@@ -9,6 +9,7 @@
 -   `FontSizePicker`: Allow custom units for custom font size control ([#48468](https://github.com/WordPress/gutenberg/pull/48468)).
 -   `Navigator`: Disable initial screen animation ([#49062](https://github.com/WordPress/gutenberg/pull/49062)).
 -   `FormTokenField`: Hide suggestions list on blur event if the input value is invalid ([#48785](https://github.com/WordPress/gutenberg/pull/48785)).
+-	`AnglePickerControl`: Style to better fit in narrow contexts and improve RTL layout ([#49046](https://github.com/WordPress/gutenberg/pull/49046)).


This needs to be moved up again, there was a release a few days ago. (When will we be free from this merge annoyance? 🥲)

Good catch!

stokesman · 2023-03-17T19:10:50Z

6835eea Restores a bit of style whose removal was part of proposed changes to focus handling that were dropped from this PR. Ultimately it’s a non-change, so I'll merge without further review.

Thanks again to Joen and Lena 🙇

stokesman added Needs Design Feedback Needs general design feedback. [Package] Components /packages/components labels Mar 13, 2023

stokesman force-pushed the try/affix-knob-angle-picker-control branch from de51103 to 7c2c8e3 Compare March 16, 2023 06:25

jasmussen approved these changes Mar 16, 2023

View reviewed changes

mirka self-requested a review March 16, 2023 09:05

mirka assigned stokesman Mar 16, 2023

mirka reviewed Mar 16, 2023

View reviewed changes

packages/components/src/angle-picker-control/index.tsx Outdated Show resolved Hide resolved

stokesman force-pushed the try/affix-knob-angle-picker-control branch from 7c2c8e3 to 2fd322d Compare March 17, 2023 05:09

stokesman changed the title ~~Affix knob in AnglePickerControl~~ Fix visuals of AnglePickerControl Mar 17, 2023

stokesman marked this pull request as ready for review March 17, 2023 05:28

stokesman requested a review from ajitbohra as a code owner March 17, 2023 05:28

stokesman force-pushed the try/affix-knob-angle-picker-control branch from 2fd322d to 650b716 Compare March 17, 2023 06:00

stokesman changed the title ~~Fix visuals of AnglePickerControl~~ AngleControlPicker: Style to better fit in narrow contexts and improve RTL layout Mar 17, 2023

stokesman changed the title ~~AngleControlPicker: Style to better fit in narrow contexts and improve RTL layout~~ AnglePickerControl: Style to better fit in narrow contexts and improve RTL layout Mar 17, 2023

mirka approved these changes Mar 17, 2023

View reviewed changes

stokesman added 8 commits March 17, 2023 10:54

Move AngleCircle inside NumberControl as suffix

363a3ba

Restyle

f293a27

Have the degree symbol on the right for both RTL/LTR

aa80b37

Restyle

7667eae

Add changelog entry

13d8e1b

Correct name

601d53d

Sort changelog entry

f17783a

Restore :focus-visible rule from f02087f

6835eea

stokesman force-pushed the try/affix-knob-angle-picker-control branch from 1f9e1fe to 6835eea Compare March 17, 2023 18:09

stokesman merged commit 6434e0b into trunk Mar 17, 2023

stokesman deleted the try/affix-knob-angle-picker-control branch March 17, 2023 19:11

github-actions bot added this to the Gutenberg 15.5 milestone Mar 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`AnglePickerControl`: Style to better fit in narrow contexts and improve RTL layout #49046

`AnglePickerControl`: Style to better fit in narrow contexts and improve RTL layout #49046

stokesman commented Mar 13, 2023 •

edited

Loading

jasmussen commented Mar 14, 2023

stokesman commented Mar 14, 2023

jasmussen commented Mar 15, 2023

stokesman commented Mar 16, 2023

jasmussen left a comment

jasmussen Mar 16, 2023

jasmussen Mar 16, 2023

stokesman Mar 17, 2023

jasmussen Mar 17, 2023

jasmussen Mar 16, 2023

mirka left a comment

stokesman commented Mar 17, 2023

github-actions bot commented Mar 17, 2023 •

edited

Loading

mirka left a comment

mirka Mar 17, 2023

mirka Mar 17, 2023

stokesman Mar 17, 2023

stokesman commented Mar 17, 2023

AnglePickerControl: Style to better fit in narrow contexts and improve RTL layout #49046

AnglePickerControl: Style to better fit in narrow contexts and improve RTL layout #49046

Conversation

stokesman commented Mar 13, 2023 • edited Loading

What?

Why?

How?

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

In Post Editor adjusting a gradient

In Global Styles

Other screenshots to show design alternatives

A before/after of just moving the knob “inside” without other style changes

A style experiment I tried along the way

jasmussen commented Mar 14, 2023

stokesman commented Mar 14, 2023

jasmussen commented Mar 15, 2023

stokesman commented Mar 16, 2023

jasmussen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mirka left a comment

Choose a reason for hiding this comment

stokesman commented Mar 17, 2023

github-actions bot commented Mar 17, 2023 • edited Loading

mirka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stokesman commented Mar 17, 2023

`AnglePickerControl`: Style to better fit in narrow contexts and improve RTL layout #49046

`AnglePickerControl`: Style to better fit in narrow contexts and improve RTL layout #49046

stokesman commented Mar 13, 2023 •

edited

Loading

github-actions bot commented Mar 17, 2023 •

edited

Loading