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.

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

Notification Settings: Add duration to notice after successfully saving #3254

Closed
wants to merge 1 commit into from

Conversation

ebinnion
Copy link
Contributor

Addresses part of the issue in #3203.

This "should" cause the notices to disappear after a 4 second timeout. Although, in testing, I'm still noticing that a notice will stick around. I'm not sure why this is yet. And until we figure that out, it's pretty awkward that some notices will disappear while one or more won't.

@artpi suggested that this section be refactored such that the notices are included inline with each site's settings. cc @rickybanister for thoughts on that.

To test:

  • Checkout update/me-notification-settings-notices branch
  • Go to /me/notifications
  • Update a setting for one of your site's and save.
  • Update another setting and save. (quickly)

cc @rodrigoi who I believe wrote most of notification settings.

@ebinnion ebinnion added [Status] In Progress [Feature] User & Account Settings (/me) Settings and tools for managing your WordPress.com user account. labels Feb 11, 2016
@ebinnion ebinnion self-assigned this Feb 11, 2016
@rickybanister
Copy link

@ebinnion can you fill me in a bit more on the context here—are these global notices? How will the timeout be initiated? Can they just have dismiss links instead?

@rickybanister
Copy link

Ah, after looking at the initial issue, I think they should replace each other rather than time out.

cc @mtias

@BrookeDot
Copy link
Contributor

After tested the patch, this still feels awkward showing multiple alerts on the page (even with some of them timing out. I would suggest going forward with one of the solutions that @rickybanister or @artpi suggested.

I would like to see inline alerts where we auto-save when checking/unchecking a box instead of having to click "Save Settings" button but that may be out of the scope of this alert.

@ebinnion ebinnion closed this May 20, 2016
@ebinnion ebinnion deleted the update/me-notification-settings-notices branch May 20, 2016 04:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] User & Account Settings (/me) Settings and tools for managing your WordPress.com user account.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants