12647 fix css color picker #12747

kadencewp · 2018-12-09T22:08:27Z

Description

Tweaked CSS padding for input fields within the colorPicker

Fixes: #12647

How has this been tested?

Tested Manually

Screenshots

Before CSS Change:

After CSS Change:

Types of changes

Added specific CSS styling to the input fields

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.

jasmussen

I could not reproduce this issue in any browser or on Windows, for the stock Paragraph color picker:

However, the issue seems legit, and the code change (per above comments addressed) seems fine. I can imagine this issue appears primarily when using the picker in other contexts, so good to have. Thanks!

jasmussen · 2018-12-17T08:15:54Z

packages/components/src/color-picker/style.scss

@@ -199,6 +199,9 @@
 	fieldset {
 		flex: 1;
 	}


Can you add a linebreak after this?

I've added this to the latest

jasmussen · 2018-12-17T08:16:51Z

packages/components/src/color-picker/style.scss

@@ -35,7 +35,7 @@
 	position: relative;
 }
 .components-color-picker__body {
-	padding: 16px 16px 12px;
+	padding: 16px 0 12px;


While we're here, can you use grid units? Essentially the following instead:

padding: $grid-size-large 0 #{ $grid-size-small * 3 };

I've added this to the latest pull

kadencewp · 2018-12-18T00:49:12Z

Just for clarification, the reason you are not seeing this with the paragraph block is that it doesn't use the ColorPicker component, it's using PanelColorSettings.

gziolo · 2019-01-25T23:01:19Z

@kadencethemes can you rebase this PR so we could land this changes in the next plugin release?

kadencewp · 2019-01-25T23:59:20Z

Hey @gziolo I'm not very familiar with what "rebase this PR" means. Could you tell me what I need to do?

gziolo · 2019-01-28T10:19:34Z

@kadencethemes some more details about rebasing a Pull Request you can find in https://github.com/edx/edx-platform/wiki/How-to-Rebase-a-Pull-Request.

When many different people are working on a project simultaneously, pull requests can go stale quickly. A "stale" pull request is one that is no longer up to date with the main line of development, and it needs to be updated before it can be merged into the project.

When working by yourself, it is best to use git pull --rebase master, but if you're pushing to a shared repo, it is best to not do any merging or rebasing until the feature is ready for final testing, and then do a rebase at the very end. This is one reason why it is important to open pull requests whenever you have working code.

If you have a Pull Request branch that cannot be merged into master due to a conflict (this can happen for long-running Pull Request discussions), it's still best to rebase the branch (rather than merge) and resolve any conflicts on your local copy.

Once you have resolved any conflicts locally you can update the Pull Request with git push --force-with-lease.

gziolo · 2019-01-28T10:56:52Z

I also opened #13534 to add a similar note to docs so it was easier to reference in the future if someone wants to learn about rebasing PRs.

kadencewp · 2019-01-28T20:17:53Z

I'm not sure I did that all correct so let me know.

kadencewp · 2019-01-29T06:36:01Z

ok well, I see I did do this wrong, I somehow have 787 file changes. At this point, I messed around for a while trying to undo the merge I apparently did. I can't seem to get that to happen. My experience with all of this is limited, I haven't worked with a large shared project and I really only use Sourcetree on a regular basis for simple version control of my own projects. Would you suggest I simply recreate a pull with this very simple and basic CSS change to the color picker component?

You mentioned: "This is one reason why it is important to open pull requests whenever you have working code." and when this was pulled in it was working and was tested. How do I prevent this in the future if I want to help?

jasmussen · 2019-01-29T07:57:25Z

@gziolo my hero — I can offer you an emoji medal if you can help out in this PR with a good rebase to fix this up. Do you have time?

gziolo · 2019-01-29T09:38:58Z

Yes, I see that something went wrong. We are still working on #13534 to provide docs which will cover the case when someone is using a fork of Gutenberg which is the case here. For the time being, I can share the reference to the GitHub docs: https://help.github.com/articles/syncing-a-fork/. We hope to have something more targeted for your use case later this week 👍

gziolo · 2019-01-31T10:30:46Z

Sidebar Tutorial URL is wrong, should be .md

@manuelmeister It isn't part of this PR, so I would suggest opening another PR with a fix proposed.

@kadencethemes - can you check newly added docs about Git workflow: https://github.com/WordPress/gutenberg/blob/master/docs/contributors/git-workflow.md

If you can't fix it, it's fine to close this PR and open a follow up with only your changes applied on the up to date fork of Gutenberg.

kadencewp · 2019-02-01T19:37:10Z

Well well, looks like there is hope for me :) I think I got it now.

gziolo · 2019-02-01T21:13:30Z

Yeah, it’s fixed. Great success 🎉❤️

I hope that new documentation was helpful. If there is anything specific you feel we should add, let us know.

I will test this PR on Monday!

kadencewp · 2019-02-01T21:53:01Z

I would suggest adding steps for users more accustomed to sourcetree.

gziolo

It tests well. Thank you for your contribution and fixing this bug. Much appreciated 💯

Also, thank you for being so patient as you had to wait a bit to get this merged and waste some times to refresh this PR 🥇

gziolo · 2019-02-04T11:20:57Z

I would suggest adding steps for users more accustomed to sourcetree.

That would be good, right. I had to use GitHub desktop last week to update on of the PRs from the forked repository and merged it using code copied over to my own branch. It has just worked but it would be hard to explain how :)

…rnmobile/372-use-RichText-on-Title-block * 'master' of https://github.com/WordPress/gutenberg: (22 commits) Make the modal title styling consistent (#13669) Disable navigation block for text mode. (#12185) Fix: Linting problem in modal example code (#13671) Add myself as a code owner to the annotations (#13672) Add more reviewers to CODEOWNERS.md file (#13667) Plugin: Remove jQuery heartbeat-to-hooks proxying (#13576) Workflow: Add repository CODEOWNERS file (#13604) Add a mobile minimum size for form fields (#13639) Update edit-save documentation (#13578) Alt image setting (#13631) Fix: Allow years lower than 1970 in DateTime component. (#13602) Using addQueryArgs to generate Manage All Reusable Blocks link (#13653) Bump plugin version to 5.0.0-rc.1 (#13652) Update lodash to 4.17.10 (#13651) Refreshed PR (#9469) Set default values of the width and height input fields according to the actual image dimensions (#7687) 12647 fix css color picker (#12747) Remove "we" from messages (#13644) Fix: Font size picker max width on mobile (#13264) Fix/issue 12501 menu item aria label ...

* Update css to fix padding with colorPicker * Focus on to numbers * Requested changes

gziolo requested a review from jasmussen December 14, 2018 13:01

gziolo added Needs Design Feedback Needs general design feedback. [Type] Bug An existing feature does not function as intended [Package] Components /packages/components labels Dec 14, 2018

jasmussen approved these changes Dec 17, 2018

View reviewed changes

gziolo added this to the 5.0 (Gutenberg) milestone Jan 25, 2019

gziolo mentioned this pull request Jan 28, 2019

Docs: Add clarification about git workflow #13534

Merged

kadencewp added 3 commits February 1, 2019 12:12

Update css to fix padding with colorPicker

b68865f

Focus on to numbers

ba4ddd2

Requested changes

cc79501

gziolo modified the milestones: 5.0 (Gutenberg), 5.1 (Gutenberg) Feb 4, 2019

gziolo approved these changes Feb 4, 2019

View reviewed changes

gziolo merged commit dc2c1f7 into WordPress:master Feb 4, 2019

noisysocks mentioned this pull request Feb 18, 2019

Bump plugin version to 5.1.0-rc.1 #13908

Merged

noisysocks mentioned this pull request Feb 20, 2019

Bump plugin version to 5.1.0 #13960

Merged

youknowriad pushed a commit that referenced this pull request Mar 6, 2019

12647 fix css color picker (#12747)

6f4cbb1

* Update css to fix padding with colorPicker * Focus on to numbers * Requested changes

youknowriad pushed a commit that referenced this pull request Mar 6, 2019

12647 fix css color picker (#12747)

43d5f50

* Update css to fix padding with colorPicker * Focus on to numbers * Requested changes

This was referenced Jul 16, 2024

[Snyk] Upgrade @wordpress/postcss-plugins-preset from 1.6.0 to 5.1.0 WontonSam/woocommerce-admin#124

Open

[Snyk] Upgrade @wordpress/base-styles from 3.6.0 to 5.1.0 WontonSam/woocommerce-admin#127

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

12647 fix css color picker #12747

12647 fix css color picker #12747

kadencewp commented Dec 9, 2018

jasmussen left a comment

jasmussen Dec 17, 2018

kadencewp Dec 18, 2018

jasmussen Dec 17, 2018

kadencewp Dec 18, 2018

kadencewp commented Dec 18, 2018

gziolo commented Jan 25, 2019

kadencewp commented Jan 25, 2019

gziolo commented Jan 28, 2019

gziolo commented Jan 28, 2019

kadencewp commented Jan 28, 2019

kadencewp commented Jan 29, 2019

jasmussen commented Jan 29, 2019

gziolo commented Jan 29, 2019

gziolo commented Jan 31, 2019

kadencewp commented Feb 1, 2019

gziolo commented Feb 1, 2019

kadencewp commented Feb 1, 2019

gziolo left a comment •

edited

Loading

gziolo commented Feb 4, 2019

12647 fix css color picker #12747

12647 fix css color picker #12747

Conversation

kadencewp commented Dec 9, 2018

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

jasmussen left a comment

Choose a reason for hiding this comment

jasmussen Dec 17, 2018

Choose a reason for hiding this comment

kadencewp Dec 18, 2018

Choose a reason for hiding this comment

jasmussen Dec 17, 2018

Choose a reason for hiding this comment

kadencewp Dec 18, 2018

Choose a reason for hiding this comment

kadencewp commented Dec 18, 2018

gziolo commented Jan 25, 2019

kadencewp commented Jan 25, 2019

gziolo commented Jan 28, 2019

gziolo commented Jan 28, 2019

kadencewp commented Jan 28, 2019

kadencewp commented Jan 29, 2019

jasmussen commented Jan 29, 2019

gziolo commented Jan 29, 2019

gziolo commented Jan 31, 2019

kadencewp commented Feb 1, 2019

gziolo commented Feb 1, 2019

kadencewp commented Feb 1, 2019

gziolo left a comment • edited Loading

Choose a reason for hiding this comment

gziolo commented Feb 4, 2019

gziolo left a comment •

edited

Loading