Skip to content

Settings UI: Refactor ProStatus component to be more DRY and readable#6739

Merged
eliorivero merged 2 commits intofeature/settings-overhaulfrom
refactor/pro-status
Mar 24, 2017
Merged

Settings UI: Refactor ProStatus component to be more DRY and readable#6739
eliorivero merged 2 commits intofeature/settings-overhaulfrom
refactor/pro-status

Conversation

@dereksmart
Copy link
Copy Markdown
Contributor

This component because pretty convoluted.

It also fixes inconsistencies with the Akismet card in the dashboard. Now we are showing the same upgrade notice that we use for scans instead of the button. It also links the invalid key notice to an actual URL. Before it wasn't.

To test:

  • Make sure there are no regressions in the pro status cards, especially in the AAG dashboard.
  • Make sure you still see set up in the scan card within /security when it's not set up on a paid plan.

@dereksmart dereksmart added Admin Page React-powered dashboard under the Jetpack menu [Status] Needs Review This PR is ready for review. Enhancement Changes to an existing feature — removing, adding, or changing parts of it labels Mar 24, 2017
@samhotchkiss samhotchkiss modified the milestone: Settings UI Mar 24, 2017
@eliorivero
Copy link
Copy Markdown
Contributor

I put my site in a free plan and I still see the "Secure" badge in Scan settings card:

captura de pantalla 2017-03-24 a las 16 05 07

@eliorivero eliorivero 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 Review This PR is ready for review. labels Mar 24, 2017
@dereksmart
Copy link
Copy Markdown
Contributor Author

@eliorivero becuase you have vp installed and active probably. Remember that it's possible someone has VaultPress without a plan.

@dereksmart dereksmart added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Mar 24, 2017
if ( this.props.fetchingSiteData ) {
return '';
const akismetData = this.props.getAkismetData();
if ( 'invalid_key' === akismetData ) {
Copy link
Copy Markdown
Contributor

@eliorivero eliorivero Mar 24, 2017

Choose a reason for hiding this comment

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

This should use isAkismetKeyValid() since it returns only the data necessary for key validation. getAkismetData() returns all the data for prevented spam and it's a larger amount of data.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed in 304143a

@eliorivero
Copy link
Copy Markdown
Contributor

Ok, it works when VP is inactive and the SET UP button takes me to VP setup.

@eliorivero eliorivero merged commit 9e55ec1 into feature/settings-overhaul Mar 24, 2017
@eliorivero eliorivero deleted the refactor/pro-status branch March 24, 2017 21:24
@eliorivero eliorivero removed the [Status] Needs Review This PR is ready for review. label Mar 24, 2017
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 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.

3 participants