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 gradient RGBA/HSLA inputs' width #24214

Merged
merged 4 commits into from
Jul 27, 2020
Merged

Conversation

ntsekouras
Copy link
Contributor

Description

Fixes: #24201

When adding a gradient with RGBA or HSLA color values to a block, the increment buttons of number inputs cover up the inputs' values.

Steps to reproduce and screenshots in the issue.

@ntsekouras ntsekouras added [Type] Bug An existing feature does not function as intended Needs Design Feedback Needs general design feedback. [Package] Components /packages/components Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Jul 27, 2020
@ntsekouras ntsekouras self-assigned this Jul 27, 2020
@ntsekouras ntsekouras requested a review from jasmussen July 27, 2020 09:54
@@ -178,7 +178,7 @@ export class Inputs extends Component {
return (
<fieldset>
<VisuallyHidden as="legend">
{ __( 'Color value in RGB' ) }
{ __( 'Color value in RGBA' ) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think this should stay "RGB" unless disableAlpha is false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. I missed that. Thanks!

@@ -231,7 +231,7 @@ export class Inputs extends Component {
return (
<fieldset>
<VisuallyHidden as="legend">
{ __( 'Color value in HSL' ) }
{ __( 'Color value in HSLA' ) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

@github-actions
Copy link

github-actions bot commented Jul 27, 2020

Size Change: +198 B (0%)

Total Size: 1.16 MB

Filename Size Change
build/block-directory/index.min.js 7.93 kB -2 B (0%)
build/block-editor/index.min.js 125 kB +4 B (0%)
build/block-library/index.min.js 132 kB +48 B (0%)
build/blocks/index.min.js 48.2 kB -3 B (0%)
build/components/index.min.js 200 kB +30 B (0%)
build/components/style-rtl.css 15.7 kB +11 B (0%)
build/components/style.css 15.7 kB +12 B (0%)
build/data/index.min.js 8.45 kB -3 B (0%)
build/edit-navigation/index.min.js 10.8 kB -2 B (0%)
build/edit-post/index.min.js 304 kB +19 B (0%)
build/edit-site/index.min.js 17 kB +37 B (0%)
build/edit-widgets/index.min.js 9.38 kB +7 B (0%)
build/editor/index.min.js 45.3 kB +24 B (0%)
build/keyboard-shortcuts/index.min.js 2.52 kB +11 B (0%)
build/list-reusable-blocks/index.min.js 3.11 kB -2 B (0%)
build/nux/index.min.js 3.4 kB -5 B (0%)
build/plugins/index.min.js 2.56 kB -1 B
build/primitives/index.min.js 1.41 kB +4 B (0%)
build/rich-text/index.min.js 13.9 kB +9 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.min.js 1.14 kB 0 B
build/annotations/index.min.js 3.67 kB 0 B
build/api-fetch/index.min.js 3.44 kB 0 B
build/autop/index.min.js 2.82 kB 0 B
build/blob/index.min.js 620 B 0 B
build/block-directory/style-rtl.css 953 B 0 B
build/block-directory/style.css 952 B 0 B
build/block-editor/style-rtl.css 10.8 kB 0 B
build/block-editor/style.css 10.8 kB 0 B
build/block-library/editor-rtl.css 7.63 kB 0 B
build/block-library/editor.css 7.63 kB 0 B
build/block-library/style-rtl.css 7.83 kB 0 B
build/block-library/style.css 7.83 kB 0 B
build/block-library/theme-rtl.css 728 B 0 B
build/block-library/theme.css 729 B 0 B
build/block-serialization-default-parser/index.min.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.min.js 3.1 kB 0 B
build/compose/index.min.js 9.67 kB 0 B
build/core-data/index.min.js 11.5 kB 0 B
build/data-controls/index.min.js 1.29 kB 0 B
build/date/index.min.js 5.38 kB 0 B
build/deprecated/index.min.js 772 B 0 B
build/dom-ready/index.min.js 568 B 0 B
build/dom/index.min.js 3.23 kB 0 B
build/edit-navigation/style-rtl.css 1.08 kB 0 B
build/edit-navigation/style.css 1.08 kB 0 B
build/edit-post/style-rtl.css 5.61 kB 0 B
build/edit-post/style.css 5.61 kB 0 B
build/edit-site/style-rtl.css 3.06 kB 0 B
build/edit-site/style.css 3.06 kB 0 B
build/edit-widgets/style-rtl.css 2.45 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/style-rtl.css 3.8 kB 0 B
build/editor/style.css 3.79 kB 0 B
build/element/index.min.js 4.65 kB 0 B
build/escape-html/index.min.js 733 B 0 B
build/format-library/index.min.js 7.72 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.min.js 2.13 kB 0 B
build/html-entities/index.min.js 621 B 0 B
build/i18n/index.min.js 3.56 kB 0 B
build/is-shallow-equal/index.min.js 711 B 0 B
build/keycodes/index.min.js 1.94 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.min.js 5.33 kB 0 B
build/notices/index.min.js 1.79 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/priority-queue/index.min.js 789 B 0 B
build/redux-routine/index.min.js 2.85 kB 0 B
build/server-side-render/index.min.js 2.71 kB 0 B
build/shortcode/index.min.js 1.7 kB 0 B
build/token-list/index.min.js 1.27 kB 0 B
build/url/index.min.js 4.06 kB 0 B
build/viewport/index.min.js 1.85 kB 0 B
build/warning/index.min.js 1.14 kB 0 B
build/wordcount/index.min.js 1.17 kB 0 B

compressed-size-action

@@ -227,6 +227,11 @@
padding-top: 16px;
display: flex;
align-items: flex-end;
min-width: 22em;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we use "em"s here? It seems like we prefer pixels in general in Gutenberg?

Copy link
Contributor

Choose a reason for hiding this comment

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

It is still possible to have the issuue on mobile
Capture d’écran 2020-07-27 à 11 21 09 AM

I wonder if a better fix is to wrap inputs (return to the line) when they reach a certain size instead of increasing the popover size.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder if a better fix is to wrap inputs (return to the line)

What do you mean by that?

It seems like we prefer pixels in general in Gutenberg

Okay, I'll use pixels.

Issue on mobile

Is this the min width we should make sure it's shown properly? $break-mobile: 480px;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've made the changes to be visible in mobile too with the minimum min-width required.

I wonder if a better fix is to wrap inputs (return to the line) when they reach a certain size instead of increasing the popover size.

@youknowriad just let me know what did you mean, so I can maybe explore a different approach.

If you meant though to listen to input changes and change layout or resize, I believe it would cause an unpleasant/confusing user experience.

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

I think this is a good fix for the moment.
I was thinking maybe if the width is not enough to show 4 inputs, move one to the row (flex-wrap) but it could be explored later.

@ntsekouras ntsekouras merged commit 9fe5d0d into master Jul 27, 2020
@ntsekouras ntsekouras deleted the fix/gradient-rgba-input-widths branch July 27, 2020 12:38
@github-actions github-actions bot added this to the Gutenberg 8.7 milestone Jul 27, 2020
youknowriad pushed a commit that referenced this pull request Jul 27, 2020
* fix gradient rgba/hsla inputs width

* fix label by disableAlpha value

* change to px values

* css changes
@youknowriad youknowriad removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Jul 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Design Feedback Needs general design feedback. [Package] Components /packages/components [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gradient RGBA/HSLA inputs are not wide enough in WordPress 5.5
2 participants