-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Hide custom font size label and option from font size picker select #18049
Hide custom font size label and option from font size picker select #18049
Conversation
Do, I need to change anything in my commit ? I see eslint & javascript mobile test is failed, can anyone guide ? |
@Waqas-Ali-Azhar Thank you for your contribution! It's always exciting to see someone starting to contribute to WordPress. It does look like there are some minor linting issues that need to be fixed in the PR. For any of the reports, you can click on "Details" for more information and then click on the name of the failing test (In this case, job 42766.1 Lint) for the specifics. The other failing test "javascript mobile" can likely be ignored if the things that are failing are not related to the changes you made, which looks like the case here. To make your life easier, instead of making the changes and submitting them back and waiting for Travis to run the lint test, you can run The linting rules are usually fairly self-explanatory, but the coding standards can help with any question: https://github.com/WordPress/gutenberg/blob/master/docs/contributors/coding-guidelines.md. Also, please feel free to ask here if anything is still unclear, I'm happy to help! |
Thank you @brentswisher i am very excited for this one to be merged as its my first PR and first one for Hacktoberfest. So to be merged my next step is to run lint command locally and resolve the issue and generate a new PR ? |
@Waqas-Ali-Azhar That's awesome, Hacktoberfest is great! To fix the linting issues, you will want to check out the branch you created for this PR, fix the issues and verify them with |
return [ | ||
...optionsArray.map( ( option ) => ( { value: option.slug, label: option.name } ) ) | ||
]; | ||
} | ||
return [ | ||
...optionsArray.map( ( option ) => ( { value: option.slug, label: option.name } ) ), | ||
{ value: 'custom', label: __( 'Custom' ) }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you guard this line with the check instead or conditionally push to the object for the custom option to avoid code duplication?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, i can try that.
Hey @Waqas-Ali-Azhar, just thought I would check in with you and see how it's going, is there anything I can help you with, or any questions you might have? |
Hey @brentswisher thanks for checking on me. I appreciate and definitely will use your guidance along the way to open source contributions, I was figuring out how to use guard and prevent code duplication, i think i have come up with a solution using const and return it, i will share code shortly and contribute in this week a new PR |
Sounds great! Don't be afraid to push something up even if it isn't perfect, I'm happy to take a look and help move it forward. |
Hey @Waqas-Ali-Azhar, just checking in, is this something you think you will be able to update? Any questions/concerns I can help with? |
Just coming back to it again i did a fix could not submit that, can i share code here before submission ? |
@Waqas-Ali-Azhar I'd be happy to try and help however I can, but it really is a lot easier if you can update your branch and re-push the changes, what was the reason you couldn't submit it? It's been long enough that your branch is probably a little out of date, you can try the instructions here for keeping your fork up to date which might help |
Yeah i tried last week but there there was some ces issue coming there the dropdown control shrinks and text box expands. Let me see the work around and i will push it up. Thank You for utter support
Get Outlook for Android<https://aka.ms/ghei36>
…________________________________
From: Brent Swisher <[email protected]>
Sent: Wednesday, April 8, 2020 5:34:05 PM
To: WordPress/gutenberg <[email protected]>
Cc: Waqas Ali Azhar <[email protected]>; Mention <[email protected]>
Subject: Re: [WordPress/gutenberg] Hide custom font size label and option from font size picker select (#18049)
@Waqas-Ali-Azhar<https://github.com/Waqas-Ali-Azhar> I'd be happy to try and help however I can, but it really is a lot easier if you can update your branch and re-push the changes, what was the reason you couldn't submit it? It's been long enough that your branch is probably a little out of date, you can try the instructions here for keeping your fork up to date<https://github.com/WordPress/gutenberg/blob/master/docs/contributors/git-workflow.md> which might help
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#18049 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACZDKSX4MP5WL5BBDODTUBLRLRVL3ANCNFSM4JC6DGPA>.
|
@brentswisher i would be happy if you can help me, i lost my hardrive which contained all code, now it looks that main repository is a way ahead of time i forked it, can you put me to direction what i need to continue this pull request and clean my code please |
thanks it will definitely help |
@brentswisher and @Waqas-Ali-Azhar how is the PR coming along? I noticed the last comment mentioned an error. Has this been taken care of? How can we help move the PR forward. Is the PR still valid? @aristath Perhaps this is a PR for you to check on? |
Reviewed in today's triage and it looks like this feature was added in #18842 so I'm going to close this PR. @Waqas-Ali-Azhar thanks for the contribution, unfortunately this PR got stuck before being merged. Please do look for an additional issues and I also would be glad to help you. |
Description
Fixes #17941
This PR solves removes Custom as an option from font-size-picker, a fully accessible and customizable select component with font sizes, in the @wordpress/components package.
It works well, if you call function
add_theme_support('disable-custom-font-sizes')
inside functions.php of the active theme, it hides custom size option inside font size picker select component on gutenberg paragraph block on post or page add or edit screens.How has this been tested?
Blank installation of WP 5.2.3 and TwentyNineteen theme.
Install and enable Gutenberg forked from WordPress/gutenberg
Added patch
Run
npm install
,npm run-script build
on command line inside gutenberg plugin folderGoto post add screen and add a paragraph block
Add or remove
add_theme_support('disable-custom-font-sizes')
in functions.php of active themeCustom font size option will be removed or shown respectively
Tested on post, page add new and edit menus, updated successfully
Screenshots
Types of changes
Bug fix ( Disabling custom font sizes does not remove 'custom' option from Paragraph block font size dropdown )
Checklist: