Skip to content

Jetpack Notices: fix ugly VaultPress notice#6963

Merged
zinigor merged 1 commit intomasterfrom
fix/double-vp-notice
Apr 14, 2017
Merged

Jetpack Notices: fix ugly VaultPress notice#6963
zinigor merged 1 commit intomasterfrom
fix/double-vp-notice

Conversation

@dereksmart
Copy link
Copy Markdown
Contributor

Fixes #6960

Manually remove the link to the VaultPress, so there's not two links anymore.

No more action notices.

Added an icon.

Before:
image

After:
screen shot 2017-04-11 at 1 05 54 pm

@dereksmart dereksmart added Admin Page React-powered dashboard under the Jetpack menu [Status] Needs Design Review Design has been added. Needs a review! [Status] Needs Review This PR is ready for review. Enhancement Changes to an existing feature — removing, adding, or changing parts of it labels Apr 11, 2017
@rickybanister
Copy link
Copy Markdown

This looks great. Is this notice dismissable, should it be? I guess it's a pretty big deal and would be fine to be persistent.

cc @annezazu

@rickybanister rickybanister removed the [Status] Needs Design Review Design has been added. Needs a review! label Apr 11, 2017
@annezazu
Copy link
Copy Markdown
Collaborator

Will this clean up the double errors or will this just clean up the Jetpack error or both? :)

@dereksmart
Copy link
Copy Markdown
Contributor Author

dereksmart commented Apr 11, 2017

@annezazu By double errors, do you mean that there is a notice showing in both the VP admin and the Jetpack admin? If so, it's worth pointing out that VP shows this notice also on every other page of the wp-admin, so I think showing it in Jetpack is intentional.

@annezazu
Copy link
Copy Markdown
Collaborator

It's fine to show it in Jetpack but it's a completely different design for everywhere other than JP admin. If it shows in JP admin, do we want the normal VP error to continue to show?

screen-shot-2017-04-09-at-9-46-17-am

@dereksmart
Copy link
Copy Markdown
Contributor Author

Ah ok, I think the intent described in #6960 is to use WordPress.com's styling as much as possible while we're in the Jetpack admin, but I'll let @rickybanister verify that one.

For funs, this is sort of what it would look like:
screen shot 2017-04-11 at 3 20 22 pm

@rickybanister
Copy link
Copy Markdown

Yes, ideally we deprecate any 'vaultpress' specific styling and go with all 'calypso' styled notices, even outside of the Jetpack dashboard if this will appear there. That will set us up nicely for the future.

@rickybanister
Copy link
Copy Markdown

Sorry, there was some miscommunication here.

We want two things to happen here:

  1. the yellow notice with inline links within the jetpack dashboard looks perfect. Let's ship that.
  2. if we are going to continue to show the VP notice on every page in wp-admin it needs to look exactly the same as the dops-components notice. Can we make that happen as well? If we can't let's hide the non-jetpack notice outside of Jetpack.

@dereksmart
Copy link
Copy Markdown
Contributor Author

Can we make that happen as well? If we can't let's hide the non-jetpack notice outside of Jetpack.

These things will need to be handled by the VaultPress plugin itself, so we'll need to request this on their p2.

@dereksmart dereksmart 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 Apr 12, 2017
Copy link
Copy Markdown
Member

@thingalon thingalon left a comment

Choose a reason for hiding this comment

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

Works for me :)

@zinigor zinigor merged commit 156f77d into master Apr 14, 2017
@zinigor zinigor deleted the fix/double-vp-notice branch April 14, 2017 12:19
@zinigor zinigor removed the [Status] Ready to Merge Go ahead, you can push that green button! label Apr 14, 2017
jeherve added a commit that referenced this pull request Apr 24, 2017
eliorivero pushed a commit that referenced this pull request Apr 25, 2017
* Changelog: initial commit for 4.9 release.

* Changelog: add #6929

* Changelog: move old changelogs to changelog.txt

* Readme: restore deleted release post link.

The post is now live.

* Changelog: add #6853

* Changelog: add #6856

* Changelog: add #6857

* Changelog: add #6884

* Changelog: add #6885

* Changelog: add #6892

* Changelog: add #6894

* Changelog: add #6898

* Changelog: add #6899

* Changelog: add #6900

* Changelog: add #6909

* Changelog: add #6927

* Changelog: add #6947

* Chagelog: add #6958

* Changelog: add #6961

* Changelog: add #6963

* Changelog: add #6965

* Changelog: add #6986

* Changelog: add #7000

* Changelog: add #7013

* Changelog: add #7015

* Changelog: add #7019

* Changelog: add #7028

* Changelog: add #6998

* Changelog: add #6999

* Changelog: add #7044

* Changelog: add #6881

* Changelog: add #6922

* Changelog: add #6940

* Changelog: add #6962

* Changelog: add #6942

* Changelog: add #6959

* Changelog: add #7018

* Changelog: add #6948

* Changelog: add #6657

* Changelog: add #7030

* Changelog: add #7048

* Changelog: add #7031

* Changelog: add #6990

* Changelog: add #6957

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

6 participants