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

Settings: add descriptions to settings sections #10940

Merged
merged 6 commits into from
Dec 13, 2018
Merged

Settings: add descriptions to settings sections #10940

merged 6 commits into from
Dec 13, 2018

Conversation

keoshi
Copy link
Contributor

@keoshi keoshi commented Dec 11, 2018 •
edited
Loading

Fixes #10688

Changes proposed in this Pull Request:

  • Adds descriptions to all settings sections headers explaining the theme of the options below.
  • Cleans up dependencies paths on settings/index.jsx.

image

Testing instructions:

  • Fire up this PR.
  • Go to Jetpack settings.
  • Read the descriptions on each section.

Proposed changelog entry for your changes:

  • Settings: add descriptions to settings sections

@keoshi keoshi added [Status] In Progress Admin Page React-powered dashboard under the Jetpack menu [Status] Needs Copy labels Dec 11, 2018
@keoshi keoshi self-assigned this Dec 11, 2018
@keoshi keoshi requested a review from a team as a code owner December 11, 2018 16:28
@jetpackbot
Copy link

jetpackbot commented Dec 11, 2018 •
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: January 10, 2019.
Scheduled code freeze: January 3, 2019

Generated by 🚫 dangerJS against 86e7eaf

@keoshi keoshi mentioned this pull request Dec 11, 2018
Merged
@keoshi keoshi added the DO NOT MERGE don't merge it! label Dec 11, 2018
@keoshi keoshi added [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! and removed DO NOT MERGE don't merge it! [Status] In Progress [Status] Needs Copy labels Dec 13, 2018
@keoshi keoshi requested a review from a team December 13, 2018 09:35
@keoshi
Copy link
Contributor Author

keoshi commented Dec 13, 2018

Note for @joanrho and @jeffgolenski: we went over the copy on p2JRYi-3Qv-p2, and since the final suggestions were so short I eliminated the redundant title (e.g. Writing Tools under the already selected Writing tab) and the paragraph, and used a big font size for the entire description instead.

@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 Dec 13, 2018
jeherve
jeherve previously approved these changes Dec 13, 2018
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.

Looking good, and works well in my tests. 👍

@keoshi
Copy link
Contributor Author

keoshi commented Dec 13, 2018

@jeffgolenski Added to to this test site, would appreciate it if you could take a look at the descriptions in the settings here: https://used-longhorn-58.jurassic.ninja/wp-admin/admin.php?page=jetpack#/settings

Thank you in advance!!

jeffgolenski
jeffgolenski previously requested changes Dec 13, 2018
View reviewed changes
Copy link
Member

@jeffgolenski jeffgolenski left a comment

Choose a reason for hiding this comment

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

@keoshi Works well for me. Can we make the test flush with the left side of the cards below it? example: https://cloudup.com/cLLjdQXwe6n

@keoshi
Copy link
Contributor Author

keoshi commented Dec 13, 2018

Yeah, I actually gave it a bit of padding to follow the tab navigation's, but I can make that adjustment.

untitled 2

@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Dec 13, 2018
@keoshi keoshi dismissed jeffgolenski’s stale review December 13, 2018 16:41

Made changes to the card's padding on 86e7eaf

@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 Dec 13, 2018
jeherve
jeherve approved these changes Dec 13, 2018
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.

Looking good. Merging!

@jeherve jeherve merged commit bc56ba4 into master Dec 13, 2018
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Dec 13, 2018
@jeherve jeherve deleted the add/header-descriptions-to-settings-sections branch December 13, 2018 20:47
jeherve pushed a commit that referenced this pull request Dec 18, 2018
@keoshi @jeherve
Add settings description
1ef419a 
See: #10940
jeherve added a commit that referenced this pull request Dec 19, 2018
@jeherve
Changelog: add #10940
526caf9
jeherve added a commit that referenced this pull request Jan 3, 2019
@jeherve
Changelog: add #10940
768eb33
jeherve added a commit that referenced this pull request Jan 3, 2019
@jeherve
Jetpack 6.9: Changelog and testing list (#10722)
8d91603 
* Add first version of the Changelog and testing list for 6.9

* Changelog: add #10710

* changelog: add #10538

* changelog: add #10741

* changelog: add #10749

* changelog: add #10664

* changelog: add #10224

* changelog: add #10788

* Changelog: add #10560

* Chanegelog: add #10812

* changelog: add #10556

* Changelog: add #10668

* Changelog: add #10846

* Changelog: add #10947

* Changelog: add #10962

* Changelog: add #10956

* Changelog: add #10940

* Changelog: add #10934

* Changelog: add #10912

* changelog: add #10866

* changelog: add #10924

* Changelog: add #10936

* Changelog: add #10833

* changelog: add #10867

* Changelog: add #10960

* Changelog: add #10888

* changelog: add #10840

* changelog: add #10972

* Changelog: add #10979

* changelog: add #10909

* Changelog: add #10958

* Changelog: add #10981

* Changelog: add #10564

* Changelog: add #10809

* Changelog: add #10982

* Changelog: add #10706

* Changelog: add #10978

* Changelog: add #10132

* Changelog: add #11022

* Changelog: add #11024

* Changelog: add #10875

* Changelog: add #11030

* Changelog: add #11053

* Changelog: add #10880

* Changelog: add #9359

* Changelog: add #11037

* Update block list

* Changelog: add #11060

* Changelog: add #10755

* changelog: add #11000

* Changelog: add #10786

* Changelog: add #10945

* Changelog: add #10597
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

@jeffgolenski jeffgolenski jeffgolenski left review comments

Assignees

@keoshi keoshi

Labels
Admin Page React-powered dashboard under the Jetpack menu
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@keoshi @jetpackbot @jeffgolenski @jeherve @matticbot