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.

Sign up for GitHub

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

Jump to bottom

Blocks: allow multi-line submit buttons #11223

Merged
merged 3 commits into from
Feb 11, 2019
Merged

Blocks: allow multi-line submit buttons #11223

merged 3 commits into from
Feb 11, 2019

Conversation

roccotripaldi
Copy link
Member

The Contact Form and Subscription form use a submit button component that allows for multiple lines. However, on the front-end, we use an <input> tag which does not support new lines within its value attribute.

This PR fixes this issue by using a <button> component instead

Fixes n/a

Changes proposed in this Pull Request:

  • This PR changes the way the subscription form and contact form are displayed on the front end.

Testing instructions:

  • Try running this PR on Jurassic.ninja
  • Try creating a subscription form block in the post editor. Create a multi-line submit button and ensure:
    • The multi-lines appear when you preview the post
    • You can still use the form to subscribe to the blog

screen shot 2019-01-29 at 4 13 59 pm

  • Try creating a contact form block in the post editor. Create a multi-line submit button and ensure:
    • The multi-lines appear when you preview the post
    • You can still use the form to contact the blog owner

screen shot 2019-01-29 at 4 17 13 pm

Proposed changelog entry for your changes:

  • Fixes an issue with the submit button in the contact form and subscription form blocks.

@roccotripaldi roccotripaldi added [Feature] Contact Form [Status] Needs Review To request a review from Crew. Label will be renamed soon. [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Block] Subscriptions labels Jan 29, 2019
@roccotripaldi roccotripaldi added this to the 7.1 milestone Jan 29, 2019
@roccotripaldi roccotripaldi requested a review from a team January 29, 2019 21:18
@roccotripaldi roccotripaldi requested a review from a team as a code owner January 29, 2019 21:18
@matticbot
Copy link
Contributor

D23643-code. (newly created revision)

enejb
enejb reviewed Jan 29, 2019
View reviewed changes
modules/subscriptions/views.php Outdated
<?php if ( ! empty( $submit_button_classes ) ) { ?>
class="<?php echo esc_attr( $submit_button_classes ); ?>"
<?php }; ?>
<?php if ( ! empty( $submit_button_styles ) ) { ?>
style="<?php echo esc_attr( $submit_button_styles ); ?>"
<?php }; ?>
name="jetpack_subscriptions_widget"
/>
>
<?php echo wp_kses( $subscribe_button, array( 'br' => array() ) ); ?>
Copy link
Member

Choose a reason for hiding this comment

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

could we use $allowed_html_tags_for_submit_button ?

enejb
enejb reviewed Jan 29, 2019
View reviewed changes
modules/subscriptions/views.php Outdated
<?php if ( ! empty( $submit_button_classes ) ) { ?>
class="<?php echo esc_attr( $submit_button_classes ); ?>"
<?php }; ?>
<?php if ( ! empty( $submit_button_styles ) ) { ?>
style="<?php echo esc_attr( $submit_button_styles ); ?>"
<?php }; ?>
/>
>
<?php echo wp_kses( $subscribe_button, array( 'br' => array() ) ); ?>
Copy link
Member

Choose a reason for hiding this comment

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

Same as here $allowed_html_tags_for_submit_button?

@jetpackbot
Copy link

jetpackbot commented Jan 29, 2019 •
edited
Loading

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: February 5, 2019.
Scheduled code freeze: January 29, 2019

Generated by 🚫 dangerJS against da4b4f3

@jeherve 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 Crew. Label will be renamed soon. labels Jan 30, 2019
@roccotripaldi
Copy link
Member Author

@enejb or @jeherve where is the best place to put a constant that can be shared by multiple files, and will work in both the Jetpack env and .com env?

@jeherve
Copy link
Member

jeherve commented Feb 1, 2019

@roccotripaldi We have class.jetpack-constants.php that I believe is synced.

@jeherve jeherve added [Status] Needs Review To request a review from Crew. 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 Feb 4, 2019
@roccotripaldi roccotripaldi added the [Type] Bug When a feature is broken and / or not performing as intended label Feb 5, 2019
@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Feb 11, 2019
jeherve
jeherve approved these changes Feb 11, 2019
View reviewed changes
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This tests well for me. Merge when ready.

@lezama lezama merged commit 91e41df into master Feb 11, 2019
@ghost ghost removed [Status] Needs Changelog [Status] Ready to Merge Go ahead, you can push that green button! labels Feb 11, 2019
@lezama lezama deleted the fix/submit-button-multi-line branch February 11, 2019 14:57
@jeffersonrabb jeffersonrabb mentioned this pull request Feb 12, 2019
Merged
@artpi artpi mentioned this pull request May 21, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@enejb enejb enejb left review comments

@jeherve jeherve jeherve approved these changes

Assignees
No one assigned
Labels
[Block] Subscriptions [Feature] Contact Form [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack Touches WP.com Files [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Milestone
7.1
Development

Successfully merging this pull request may close these issues.
6 participants
@roccotripaldi @matticbot @jetpackbot @jeherve @enejb @lezama