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

Remove inline learn more prompts on comments section #12301

Merged
merged 2 commits into from
May 16, 2019
Merged

Remove inline learn more prompts on comments section #12301

merged 2 commits into from
May 16, 2019

Conversation

scottsweb
Copy link
Contributor

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 [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review To request a review from Crew. Label will be renamed soon. [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 as a code owner May 8, 2019 16:13
@jetpackbot
Copy link

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
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 To request a review from Crew. Label will be renamed soon. 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
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
Contributor

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

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

@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 Copy Review Copy has been added. Marketing will be notified for a copy review. labels May 16, 2019
@scottsweb scottsweb dismissed stale reviews from MichaelArestad and jeherve via f4f348c May 16, 2019 09:14
@scottsweb scottsweb added the [Status] Needs Review To request a review from Crew. Label will be renamed soon. label May 16, 2019
@scottsweb
Copy link
Contributor Author

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 removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels May 16, 2019
@jeherve jeherve added the [Status] Ready to Merge Go ahead, you can push that green button! label May 16, 2019
jeherve
jeherve approved these changes May 16, 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.

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

@MichaelArestad MichaelArestad MichaelArestad left review comments

Assignees

@scottsweb scottsweb

Labels
Admin Page React-powered dashboard under the Jetpack menu [Status] Design Review Complete [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
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