Skip to content

Conversation

@eliorivero
Copy link
Contributor

@eliorivero eliorivero commented Mar 23, 2018

Fixes #8922

Changes proposed in this Pull Request:

  • allow to display a short text and a link in the info popover. The support property of SettingsGroup component is no longer a string and no longer receives a URL. It receives and object with text and link properties, that are the text blurb and the link to the support doc respectively.
  • add support text & link to all settings group in Writing tab. Introduce new settings group for portfolios in custom content types. Refactor theme enhancements to remove unnecessary code
  • add support text & link to all settings group in the Sharing, Discussion, Traffic, and Security tabs.
  • align popover towards left to follow Calypso's style

Popovers now look like this with the text and link

captura de pantalla 2018-03-26 a la s 14 02 25

Testing instructions:

  • verify that on each settings card the text corresponds to the one provided here
  • ensure that the support link is correct

Proposed changelog entry for your changes:

  • Settings in Jetpack dashboard now feature contextual help and a link to learn more about it.

@eliorivero eliorivero added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it Admin Page React-powered dashboard under the Jetpack menu labels Mar 23, 2018
@eliorivero eliorivero self-assigned this Mar 23, 2018
@eliorivero eliorivero requested a review from a team as a code owner March 23, 2018 17:14
@ghost ghost removed the [Status] In Progress label Mar 23, 2018
@eliorivero eliorivero force-pushed the update/learn-more-info branch from 982509a to 4865f26 Compare March 23, 2018 18:20

SettingsGroup.propTypes = {
support: PropTypes.string,
// support: PropTypes.string,
Copy link
Contributor

Choose a reason for hiding this comment

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

👀

@oskosk oskosk added this to the 6.0 milestone Mar 23, 2018
@eliorivero eliorivero added [Status] Needs Review This PR is ready for review. [Status] In Progress and removed [Status] Needs Review This PR is ready for review. labels Mar 23, 2018
@eliorivero
Copy link
Contributor Author

@rickybanister since @nicoleckohler suggested that we should add another info popover for portfolios, I added it and wanted to know if it's ok that it looks like this now or if we should keep the previous appearance

Before

captura de pantalla 2018-03-23 a la s 13 23 29

After

captura de pantalla 2018-03-23 a la s 13 23 37

@rickybanister
Copy link

Yes indeed, splitting it is great.

@rickybanister
Copy link

Will you able to follow up the jetpack prs with similar changes in calypso?

@eliorivero
Copy link
Contributor Author

Sure, I will 👍

@rickybanister
Copy link

❤️

…nk provided, it verifies that no link will be rendered
@eliorivero eliorivero force-pushed the update/learn-more-info branch from 0862e2d to f648d5c Compare March 26, 2018 16:55
Copy link
Contributor

@oskosk oskosk left a comment

Choose a reason for hiding this comment

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

LGTM!

@oskosk oskosk added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Mar 26, 2018
Copy link
Contributor

@oskosk oskosk left a comment

Choose a reason for hiding this comment

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

LGTM!

@oskosk oskosk merged commit 439b734 into master Mar 26, 2018
@ghost ghost removed [Status] Needs Changelog [Status] Ready to Merge Go ahead, you can push that green button! labels Mar 26, 2018
@oskosk oskosk deleted the update/learn-more-info branch March 26, 2018 17:33
oskosk added a commit that referenced this pull request Mar 26, 2018
dereksmart pushed a commit that referenced this pull request Mar 27, 2018
* Changelog 6.0: create base for changelog.

* Add #8938 to changelog

* Add #8962 to changelog

* Add #8974 to changelog

* Add #8975 to changelog

* Add #8978 to changelog

* Add #8867 to changelog

* Add #8937 to changelog

* Add #8961 to changelog

* Add #8855 to changelog

* Add #8944 to changelog

* Add #8973 to changelog

* Add #8977 to changelog

* Add #8979 to changelog

* Add #8980 to changelog

* Add #8982 to changelog

* Add #8983 to changelog

* Add #8984 to changelog

* Add #8986 to changelog

* Add #9005 to changelog

* Add #9010 to changelog

* Add #9012 to changelog

* Add #9021 to changelog

* Add #9022 to changelog

* Add #9056 to changelog

* Add #9061 to changelog

* Add #9079 to changelog

* Add #9080 to changelog

* Add #9088 to changelog

* Add #9096 to changelog

* Add #9097 to changelog

* Add #9100 to changelog

* Add #9107 to changelog

* Add #8969 to changelog

* Add #8993 to changelog

* Add #9003 to changelog

* Add #9031 to changelog

* Add #8945 to changelog

* Add #9052 to changelog

* Add #9058 to changelog

* Add #9066 to changelog

* Add #9076 to changelog

* Add #9053 to changelog

* Add #9108 to changelog

* Add #9135 to changelog

* Add #9148 to changelog

* Add #9125 to changelog

* Add #9137 to changelog

* Added testing instructions for 6.0.

* Added IS testing instructions, huge props to @tiagonoronha.

* Added #8498 to changelog.

* Added #8954 to changelog.

* Added #8985 to changelog.

* add #9027

* add #9112 to changelog

* add #9136 to changelog

* add #9102 to changelog

* add #9093 to changelog

* add #9062 to changelog

* add #9172 to changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Admin Page React-powered dashboard under the Jetpack menu [Focus] FixTheFlows [Status] Requires String Changes [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants