Skip to content

Put the checkboxes to subscribe to comments above post comment button.#9983

Merged
abidhahmed merged 1 commit intoAutomattic:masterfrom
pattiereaves:fix-order-of-checkboxes-in-comment-form
Aug 24, 2018
Merged

Put the checkboxes to subscribe to comments above post comment button.#9983
abidhahmed merged 1 commit intoAutomattic:masterfrom
pattiereaves:fix-order-of-checkboxes-in-comment-form

Conversation

@pattiereaves
Copy link
Copy Markdown

Fixes #

Changes proposed in this Pull Request:

  • Moves the subscription checkboxes from after the submit button to before the submit button

Testing instructions:

  • When viewing the subscription options underneath the comments box on a site, the checkboxes should be over the Post Comment button.
    image

Proposed changelog entry for your changes:

Moves the subscription checkboxes from after the submit comment button to before the submit button.

@pattiereaves pattiereaves requested a review from a team as a code owner August 3, 2018 19:38
@jetpackbot
Copy link
Copy Markdown
Collaborator

Warnings
⚠️

The PR is missing at least one [Status] label. Suggestions: [Status] In Progress, [Status] Needs Review

Generated by 🚫 dangerJS

@abidhahmed abidhahmed added [Feature] Forms [Feature] Comments [Feature] Subscriptions All subscription-related things such as paid and unpaid, user management, and newsletter settings. [Status] Needs Design Review Design has been added. Needs a review! and removed [Feature] Forms labels Aug 6, 2018
@abidhahmed abidhahmed added this to the 6.5 milestone Aug 6, 2018
Copy link
Copy Markdown
Contributor

@abidhahmed abidhahmed left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@abidhahmed
Copy link
Copy Markdown
Contributor

abidhahmed commented Aug 6, 2018

Awaiting design review before proceeding further, thanks for your contribution!

@brbrr brbrr added the Enhancement Changes to an existing feature — removing, adding, or changing parts of it label Aug 7, 2018
@keoshi keoshi self-requested a review August 23, 2018 15:33
Copy link
Copy Markdown
Contributor

@keoshi keoshi left a comment

Choose a reason for hiding this comment

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

Looks good — thanks for your contribution, Pattie!

@keoshi keoshi added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Design Review Design has been added. Needs a review! labels Aug 23, 2018
@jeffgolenski
Copy link
Copy Markdown
Member

Definitely a great idea. Thanks @pattiereaves

@abidhahmed abidhahmed merged commit d7a1fad into Automattic:master Aug 24, 2018
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Aug 24, 2018
jeherve added a commit that referenced this pull request Jan 16, 2019
In #9983, we changed the way the subscription fields were hooked into the comment form.
The comment_form_after_fields action we started using is not triggered for logged in users.

comment_form_submit_button seems to be the only better filter right now.
I also added some CSS so the fields are never too close of the submit button.
jeherve added a commit that referenced this pull request Jan 25, 2019
In #9983, we changed the way the subscription fields were hooked into the comment form.
The comment_form_after_fields action we started using is not triggered for logged in users.

comment_form_submit_button seems to be the only better filter right now.
I also added some CSS so the fields are never too close of the submit button.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] Comments [Feature] Subscriptions All subscription-related things such as paid and unpaid, user management, and newsletter settings.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants