Skip to content

Remove inline learn more prompts on comments section#12301

Merged
jeherve merged 2 commits intomasterfrom
fix/learn-more-consistency
May 16, 2019
Merged

Remove inline learn more prompts on comments section#12301
jeherve merged 2 commits intomasterfrom
fix/learn-more-consistency

Conversation

@scottsweb
Copy link
Copy Markdown
Contributor

@scottsweb scottsweb commented May 8, 2019

Moves the inline learn more and privacy links into an (i) icon (support info component), creating a more consistent approach to these links as suggested in #6908

I would like a review of the copy used in the pop overs:

Show Gravatar hover cards alongside comments.'

Allow readers to use markdown in comments.

Allow readers to like individual comments.

Fixes #6908

Changes proposed in this Pull Request:

  • Remove inline 'learn more' prompts in favour of support info component

Is this a new feature or does it add/remove features to an existing part of Jetpack?

Not a new feature, I am working through old PRs with the 'Needs Design / Review' labels and making chances as necessary.

Testing instructions:

  • Go to Settings -> Discussion -> Comments
  • View the inline more links on this card
  • Pull down the new PR
  • Compare the new approach, check it all screens sizes
  • Also look for other learn more links I might have missed

Before:

Screenshot 2019-05-08 at 17 11 54

After:

Screenshot 2019-05-08 at 16 03 20
Screenshot 2019-05-08 at 16 03 28
Screenshot 2019-05-08 at 16 03 39
Screenshot 2019-05-08 at 16 03 46

Proposed changelog entry for your changes:

  • No changelog needed

@scottsweb scottsweb added Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review This PR is ready for review. [Status] Needs Design Review Design has been added. Needs a review! Admin Page React-powered dashboard under the Jetpack menu [Status] Needs Copy Review Copy has been added. Marketing will be notified for a copy review. labels May 8, 2019
@scottsweb scottsweb requested a review from a team May 8, 2019 16:13
@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented May 8, 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: June 4, 2019.
Scheduled code freeze: May 28, 2019

Generated by 🚫 dangerJS against f4f348c

jeherve
jeherve previously approved these changes May 8, 2019
View reviewed changes
Copy link
Copy Markdown
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 looks good to me. 👍

@jeherve jeherve removed the [Status] Needs Review This PR is ready for review. label May 8, 2019
@jeherve jeherve added this to the 7.4 milestone May 8, 2019
@MichaelArestad MichaelArestad self-requested a review May 15, 2019 18:39
MichaelArestad
MichaelArestad previously approved these changes May 15, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@MichaelArestad MichaelArestad left a comment

Choose a reason for hiding this comment

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

These are an improvement. I think it could be a minor improvement if, in this case the i was inline with the label (where the Learn more used to be), but this is more consistent overall.

@MichaelArestad MichaelArestad added [Status] Design Review Complete and removed [Status] Needs Design Review Design has been added. Needs a review! labels May 15, 2019
@MichaelArestad
Copy link
Copy Markdown
Contributor

MichaelArestad commented May 15, 2019

CC @michelleweber for copy review.

For Markdown, we can likely use the same tooltip copy as found on the other markdown setting in /writing:
image

@michelleweber
Copy link
Copy Markdown

michelleweber commented May 15, 2019

Hovercard should be one word. Otherwise, all looks good!

@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Copy Review Copy has been added. Marketing will be notified for a copy review. labels May 16, 2019
scottsweb
scottsweb commented May 16, 2019
View reviewed changes
@scottsweb scottsweb dismissed stale reviews from MichaelArestad and jeherve via f4f348c May 16, 2019 09:14
@scottsweb scottsweb added the [Status] Needs Review This PR is ready for review. label May 16, 2019
@scottsweb
Copy link
Copy Markdown
Contributor Author

scottsweb commented May 16, 2019

Sorry to be a pain @jeherve, can I get another approval on this. I made one small text change and it cleared your previous approval.

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. [Status] Needs Review This PR is ready for review. labels May 16, 2019
jeherve
jeherve approved these changes May 16, 2019
View reviewed changes
Copy link
Copy Markdown
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.

No worries! Approving again and merging!

@jeherve jeherve merged commit a3725f9 into master May 16, 2019
@jeherve jeherve deleted the fix/learn-more-consistency branch May 16, 2019 09:37
@matticbot matticbot removed the [Status] Ready to Merge Go ahead, you can push that green button! label May 16, 2019
jeherve added a commit that referenced this pull request May 17, 2019
@jeherve
Changelog: add #12301
b4dacd8
jeherve added a commit that referenced this pull request May 23, 2019
@jeherve
Changelog: add #12301
be5a022
jeherve added a commit that referenced this pull request May 27, 2019
@kraftbj @jeherve
7.4 Changelog (#12304)
0fb4436 
* Kick off the changelog

* Add 7.3.1

* Update date and post link

* changelog: add #12219

* changelog: add #12170

* changelog: add #12184

* Changelog: add #12268

* Changelog: add #12081

* Changelog: add #12323

* Changelog: add #12204

* Changelog: add #12269

* Changelog: add #12332

* changelog: add #12339

* changelog: add #12209

* Changelog: add #12319

* Changelog: add #12357

* Changelog: add #12124

* Changelog: add #12373

* Changelog: add #12252

* Changelog: add #12383

* Changelog: add #12372

* changelog: add #12337

* Changelog: add #12290

* Changelog: add #12301

* Changelog: add #12061

* Testing list: add instructions for #12061

* Changelog: add #12393

* Update minimum supported version

See #12287

* Changelog: add #12406

* Testing list: add #12406

* Changelog: add #12277

* Changelog: add #12412

* Changelog: add #11318

* Changelog: add #12328

* Changelog: add #12425

* Changelog: add #12380

* Changelog: add #12428

* Changelog: add #12414

* Changelog: add #12395

* Changelog & Testing list: add #12416, #12417, #12418, and #12348

* changelog: add #12379

* Changelog: add #12341

* changelog: add #12444

* Changelog: add #12434

* Changelog: add #12454

* Changelog: add #12460

* Changelog: add #12463

* Changelog: add #12457

* Changelog / testing list: add #10333

* Changelog: add #12467


Co-authored-by: Jeremy Herve <jeremy@jeremy.hu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jeherve jeherve jeherve approved these changes

+1 more reviewer

@MichaelArestad MichaelArestad MichaelArestad left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@scottsweb scottsweb

Labels

Admin Page React-powered dashboard under the Jetpack menu Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Design Review Complete

Projects

None yet

Milestone

7.4

Development

Successfully merging this pull request may close these issues.

Learn more vs (i) new window behavior

6 participants

@scottsweb @jetpackbot @MichaelArestad @michelleweber @jeherve @matticbot