Skip to content

JITM: hide message 3 seconds after clicking the module act. CTA#10960

Merged
oskosk merged 3 commits intomasterfrom
add/hide-jitm-post-activation
Dec 13, 2018
Merged

JITM: hide message 3 seconds after clicking the module act. CTA#10960
oskosk merged 3 commits intomasterfrom
add/hide-jitm-post-activation

Conversation

@jeherve
Copy link
Copy Markdown
Member

@jeherve jeherve commented Dec 12, 2018

Changes proposed in this Pull Request:

This will address part of #9462

I picked 3 seconds to give folks time to see that the button changed to an "Activated" status.

screen recording 2018-12-12 at 05 49 pm

Testing instructions:

  • Go to Jetpack > Settings and deactivate the Asset CDN.
  • Refresh the page; you should see a JITM offering you to activate the feature.
  • Click activate, and notice the message going through the different phases of the process (activating, activated), before to disappear.

Proposed changelog entry for your changes:

  • Dashboard notices: automatically dismiss notices once a feature has been activated.

@jeherve
JITM: hide message 3 seconds after clicking the module act. CTA
3f45d54 
This will address part of #9462

I picked 3 seconds to give folks time to see that the button changed to an "Activated"
status.
@jeherve jeherve added Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review This PR is ready for review. [Status] Needs Design Review Design has been added. Needs a review! [Feature] JITM Just In Time Messages - pop-up tips and suggestions that appear on the dashboard and sidebar. labels Dec 12, 2018
@jeherve jeherve self-assigned this Dec 12, 2018
@jeherve jeherve requested review from a team and oskosk December 12, 2018 16:59
@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented Dec 12, 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 cf59524

keoshi
keoshi previously approved these changes Dec 13, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@keoshi keoshi left a comment

Choose a reason for hiding this comment

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

Works great! Did you try with a fadeOut? It still causes the jump, but could be a neater way to make it go away. Just asking for curiosity, this looks good to me.

@keoshi
Copy link
Copy Markdown
Contributor

keoshi commented Dec 13, 2018

Not related to this PR, but I noticed that the site accelerator settings are not updated on-the-fly when I click this button. They still show up deactivated unless I refresh the page.

@jeherve
Copy link
Copy Markdown
Member Author

jeherve commented Dec 13, 2018

I believe they should update as well after a few seconds.

@jeherve
Copy link
Copy Markdown
Member Author

jeherve commented Dec 13, 2018

Did you try with a fadeOut?

I'll give it a try now.

@jeherve
Copy link
Copy Markdown
Member Author

jeherve commented Dec 13, 2018

@keoshi Here is how it looks after 814c0a4

screen recording 2018-12-13 at 01 09 pm

Is it better?

@keoshi
Copy link
Copy Markdown
Contributor

keoshi commented Dec 13, 2018

I kinda dig it like this, what do you think? Could even use less of a timeout since it now has the animation, but looks great!

@jeherve
Copy link
Copy Markdown
Member Author

jeherve commented Dec 13, 2018

I like it too.

Could even use less of a timeout since it now has the animation, but looks great!

Should it be 2 seconds instead of 3 then? I can make that change if needed.

@keoshi
Copy link
Copy Markdown
Contributor

keoshi commented Dec 13, 2018

@jeherve I believe the slow indicates 600 milliseconds, so 2 seconds sounds great to me!

oskosk
oskosk previously approved these changes Dec 13, 2018
View reviewed changes
Copy link
Copy Markdown
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! [Status] Design Review Complete and removed [Status] Needs Design Review Design has been added. Needs a review! [Status] Needs Review This PR is ready for review. labels Dec 13, 2018
@oskosk
Copy link
Copy Markdown
Contributor

oskosk commented Dec 13, 2018

I approved this PR but not quite clear if it's good to go with 3 seconds or 2.

@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Dec 13, 2018
@jeherve
Copy link
Copy Markdown
Member Author

jeherve commented Dec 13, 2018

I changed it to 2 seconds in cf59524.

@oskosk I'll let you merge if you like it :)

@oskosk oskosk merged commit 99ec351 into master Dec 13, 2018
@oskosk oskosk deleted the add/hide-jitm-post-activation branch December 13, 2018 21:42
jeherve added a commit that referenced this pull request Dec 19, 2018
@jeherve
Changelog: add #10960
74a0e44
jeherve added a commit that referenced this pull request Jan 3, 2019
@jeherve
Changelog: add #10960
df41917
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

@keoshi keoshi keoshi left review comments

+1 more reviewer

@oskosk oskosk oskosk approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jeherve jeherve

Labels

Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] JITM Just In Time Messages - pop-up tips and suggestions that appear on the dashboard and sidebar. [Status] Design Review Complete

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jeherve @jetpackbot @keoshi @oskosk @matticbot