Make input fields and "push button" class filterable #7614

jimmyandrade · 2017-08-09T04:50:26Z

Changes proposed in this Pull Request:

Make the class attribute of inputs and buttons filterable, so that developers can add the classes without having to hack the WordPress core or CSS framework.

Proposed changelog entry for your changes:

Now developers can use the grunion_contact_form_submit_button_class and grunion_contact_form_input_classfilters to customize the class attributes of inputs and buttons in Contact Form.

Make the `class` attribute of inputs and buttons filterable, so that developers can add the classes without having to hack the WordPress core or CSS framework.

modules/contact-form/grunion-contact-form.php

dereksmart · 2017-08-18T16:58:25Z

modules/contact-form/grunion-contact-form.php

+			 *
+			 * @param string $class Additional CSS classes for button attribute.
+			 */
+			$submit_button_class = apply_filters( 'grunion_contact_form_submit_button_class', $submit_button_class );


Let's go with jetpack_contact_form_submit_button_class for the name here

dereksmart · 2017-08-18T16:59:58Z

modules/contact-form/grunion-contact-form.php

@@ -2553,6 +2565,16 @@ function render() {
 		$field_required    = $this->get_attribute( 'required' );
 		$placeholder       = $this->get_attribute( 'placeholder' );
 		$class             = $this->get_attribute( 'class' );
+
+		/**


Since the block above has already been documented, this can just be

/** This filter is already documented in modules/contact-form/grunion-contact-form.php */

@SInCE

* Add @SInCE 5.3.0 param in code block; * Change `grunion_contact_form_submit_button_class` to `jetpack_contact_form_submit_button_class`;

jimmyandrade · 2017-10-01T16:28:20Z

@dereksmart changed :)

zinigor

Thanks for iterating with us! I have added several comments, please take a look.

zinigor · 2017-10-02T09:35:00Z

modules/contact-form/grunion-contact-form.php

+			 * Filter the contact form submit button class attribute.
+			 *
+			 * @module contact-form
+			 * @since 5.3.0


Some time has passed since this PR has been submitted, so please edit this to be 5.5.0 instead - this will only make it in the shipped code in 5.5.0.

zinigor · 2017-10-02T09:35:23Z

modules/contact-form/grunion-contact-form.php

@@ -2550,7 +2563,10 @@ function render() {
 		$field_label       = $this->get_attribute( 'label' );
 		$field_required    = $this->get_attribute( 'required' );
 		$placeholder       = $this->get_attribute( 'placeholder' );
-		$class             = 'date' === $field_type ? 'jp-contact-form-date' : $this->get_attribute( 'class' );
+    $class             = 'date' === $field_type ? 'jp-contact-form-date' : $this->get_attribute( 'class' );


Indentation is off here, please use two tabs.

zinigor · 2017-10-02T09:40:42Z

modules/contact-form/grunion-contact-form.php

-		$class             = 'date' === $field_type ? 'jp-contact-form-date' : $this->get_attribute( 'class' );
+    $class             = 'date' === $field_type ? 'jp-contact-form-date' : $this->get_attribute( 'class' );
+
+		/** This filter is already documented in modules/contact-form/grunion-contact-form.php */


Sorry, but I think you'll need to bring back the doc block here. I think @dereksmart initially thought that it was the same filter, and it's not. Also please use consistent namespacing. Would jetpack_contact_form_input_class work here instead?

zinigor · 2017-10-02T09:41:01Z

modules/contact-form/grunion-contact-form.php

@@ -1542,7 +1542,20 @@ static function parse( $attributes, $content ) {
 			$r .= "<form action='" . esc_url( $url ) . "' method='post' class='contact-form commentsblock'>\n";
 			$r .= $form->body;
 			$r .= "\t<p class='contact-submit'>\n";
-			$r .= "\t\t<input type='submit' value='" . esc_attr( $form->get_attribute( 'submit_button_text' ) ) . "' class='pushbutton-wide'/>\n";
+
+			$submit_button_class = 'pushbutton-wide';


This is a real nitpicky suggestion, but you could have avoided adding that line by just passing pushbutton-wide as a default to the apply_filters call down there.

* Removed var for `pushbutton-wide`. Now it goes straight on apply_filters; * Changed from 5.3.0 to 5.5.0 on PHPDoc block; * Indentation fix; * Removed unecessary PHPDoc block.

jimmyandrade · 2017-10-02T16:02:22Z

@dereksmart @zinigor changed :)

zinigor · 2017-10-02T18:45:29Z

Thanks for making changes, looks much better now! Only two things that are need attention now are:

the docblock for the second filter, it should be there;
the filter namespacing is inconsistent, please use the same prefix in both of them.

keoshi · 2018-06-05T12:08:54Z

@jimmyandrade Wanted to see if you missed the comment from @zinigor above. Can you please take a quick look and update the PR? Thanks in advance!

jimmyandrade · 2018-07-02T17:51:17Z

@keoshi sorry for the delay. I'm gonna do this 👍 :)

* Adds the docblock for the second filter; * Make filter namespacing consistent, using the same prefix (jetpack_) in both of them.

jetpacl -> jetpack

jimmyandrade · 2018-07-02T17:57:13Z

@keoshi @zinigor done :)

keoshi · 2018-07-03T08:40:02Z

Thank you, @jimmyandrade — pinging @zinigor so he can take another look at your PR.

jeherve

This looks good now. 🚢

Filters were updated to address the issues.

jetpackbot · 2018-09-07T11:51:12Z

	Warnings
⚠️	"Testing instructions" are missing for this PR. Please add some

This is automated check which relies on PULL_REQUEST_TEMPLATE.We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS

Filters now include a since comment.

zinigor

Thanks, looks good now!

@see

* Readme: add boilerplate for next release, 6.6 * Add 6.5 to the changelog.txt file * Set boilerplate testing list for 6.6 * Readme: update stable tag to 6.5 * Add bullets to 6.5 changelog items * Readme: add link to previous changelogs This will help folks who want to know more about past releases, while keeping the readme.txt short so as to not overwhelm translators and site owners only looking for information about the last release. * Changelog: add information at the top of the changelog file. * Changelog: add #10054 * Changelog: add #10078 * Changelog: add #10079 * Changelog: add #10064 * Changelog: add #10094 * Changelog: add #10096 * Testing list: add more information based on #10087 * Changelog: add #9847 * Changelog: add #10084 * Changelog: add #9918 * Changelog: add #7614 * Changelog: add #10116 * Changelog: add #10108 * Changelog: add #10041 * Changelog: add #10121 * Changelog: add #10134 * Changelog: add #10130 * Changelog: add #10109 * changelog: add #10137 * changelog: add #9952 * changelog: add #10120 * changelog: add #10162 * Changelog: add #10163 * Changelog: add #10092 * changelog: add #10156 * Changelog: add #10154 * changelog: add #10122 * Changelog: add #10101 * changelog: add #10105 * changelog: add #10190 * Changelog: add #10196 * changelog: add #10152 * Changelog: add #10153 * Testing list: add more details to Site Verification testing steps. @see #10143 (comment) * changelog: add #10194 * Changelog: add #10193

Fixes #7613

9b8ce6f

Make the `class` attribute of inputs and buttons filterable, so that developers can add the classes without having to hack the WordPress core or CSS framework.

jimmyandrade requested a review from georgestephanis as a code owner August 9, 2017 04:50

dereksmart previously requested changes Aug 18, 2017

View reviewed changes

dereksmart added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] Contact Form [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Aug 18, 2017

jeherve added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Sep 7, 2017

Jimmy Andrade added 2 commits October 1, 2017 13:26

PHPDoc adjustments

11dfa31

* Add @SInCE 5.3.0 param in code block; * Change `grunion_contact_form_submit_button_class` to `jetpack_contact_form_submit_button_class`;

Merge branch 'master' into patch-1

f5ff8b4

zinigor previously requested changes Oct 2, 2017

View reviewed changes

Removed unecessary lines & PHPDoc version change

41c5eb9

* Removed var for `pushbutton-wide`. Now it goes straight on apply_filters; * Changed from 5.3.0 to 5.5.0 on PHPDoc block; * Indentation fix; * Removed unecessary PHPDoc block.

jimmyandrade added 2 commits July 2, 2018 14:55

Code-style changes

a5197cf

* Adds the docblock for the second filter; * Make filter namespacing consistent, using the same prefix (jetpack_) in both of them.

Fixes prefix typo

4bdbd2b

jetpacl -> jetpack

jeherve added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Sep 4, 2018

Contact Form: update docblock version

e6f6c3b

jeherve approved these changes Sep 7, 2018

View reviewed changes

jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Sep 7, 2018

zinigor approved these changes Sep 7, 2018

View reviewed changes

jeherve merged commit 2d15c2e into Automattic:master Sep 7, 2018

matticbot added the [Status] Needs Changelog label Sep 7, 2018

ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Sep 7, 2018

jeherve added [Status] Has Changelog and removed [Status] Needs Changelog labels Sep 14, 2018

jeherve added a commit that referenced this pull request Sep 14, 2018

Changelog: add #7614

821587d

jeherve added a commit that referenced this pull request Sep 24, 2018

Changelog: add #7614

3c2b123

jimmyandrade deleted the patch-1 branch October 19, 2018 18:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make input fields and "push button" class filterable #7614

Make input fields and "push button" class filterable #7614

jimmyandrade commented Aug 9, 2017

dereksmart Aug 18, 2017

dereksmart Aug 18, 2017

jimmyandrade commented Oct 1, 2017

zinigor left a comment

zinigor Oct 2, 2017

zinigor Oct 2, 2017

zinigor Oct 2, 2017

zinigor Oct 2, 2017

jimmyandrade commented Oct 2, 2017

zinigor commented Oct 2, 2017

keoshi commented Jun 5, 2018

jimmyandrade commented Jul 2, 2018 •

edited

Loading

jimmyandrade commented Jul 2, 2018

keoshi commented Jul 3, 2018

jeherve left a comment

jetpackbot commented Sep 7, 2018

zinigor left a comment

Make input fields and "push button" class filterable #7614

Make input fields and "push button" class filterable #7614

Conversation

jimmyandrade commented Aug 9, 2017

Changes proposed in this Pull Request:

Proposed changelog entry for your changes:

dereksmart Aug 18, 2017

Choose a reason for hiding this comment

dereksmart Aug 18, 2017

Choose a reason for hiding this comment

jimmyandrade commented Oct 1, 2017

zinigor left a comment

Choose a reason for hiding this comment

zinigor Oct 2, 2017

Choose a reason for hiding this comment

zinigor Oct 2, 2017

Choose a reason for hiding this comment

zinigor Oct 2, 2017

Choose a reason for hiding this comment

zinigor Oct 2, 2017

Choose a reason for hiding this comment

jimmyandrade commented Oct 2, 2017

zinigor commented Oct 2, 2017

keoshi commented Jun 5, 2018

jimmyandrade commented Jul 2, 2018 • edited Loading

jimmyandrade commented Jul 2, 2018

keoshi commented Jul 3, 2018

jeherve left a comment

Choose a reason for hiding this comment

jetpackbot commented Sep 7, 2018

zinigor left a comment

Choose a reason for hiding this comment

jimmyandrade commented Jul 2, 2018 •

edited

Loading