Skip to content

"Dismiss all notifications" button #112

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.

Sign up for GitHub

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

Jump to bottom
Merged
nickvergessen merged 2 commits into from
Feb 2, 2018
Merged

"Dismiss all notifications" button #112

nickvergessen merged 2 commits into from
Feb 2, 2018

Conversation

@nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Jan 29, 2018 •
edited
Loading

  • Add endpoint
    • Add unit test
    • Add integration test
  • Add UI

Fix #105

@nickvergessen nickvergessen added this to the Nextcloud 14 milestone Jan 29, 2018
@MorrisJobke
Copy link
Member

MorrisJobke commented Jan 29, 2018

Code up until now looks good 👍

@nickvergessen nickvergessen changed the title Add an endpoint to delete all notifications "Dismiss all notifications" button Jan 30, 2018
@nickvergessen
Copy link
Member Author

nickvergessen commented Jan 30, 2018

@jancborchardt screenshot okay?

bildschirmfoto von 2018-01-30 12-25-15

like with the single dismiss the icon+text gets opacity 1 + pointer cursor on hover

@nickvergessen nickvergessen force-pushed the feature/105/dismiss-all-button branch from 342dd0e to ae16751 Compare January 30, 2018 11:34
@nickvergessen nickvergessen force-pushed the feature/105/dismiss-all-button branch from ae16751 to 9429570 Compare January 30, 2018 13:16
@nickvergessen
Copy link
Member Author

nickvergessen commented Jan 30, 2018

Add integration and unit tests and the UI.

Ready to review 🎉

@nickvergessen nickvergessen mentioned this pull request Jan 30, 2018
Merged
1 task
@MorrisJobke
Copy link
Member

MorrisJobke commented Feb 2, 2018

Somehow the label is not properly aligned in Safari:

bildschirmfoto 2018-02-02 um 12 31 26

Let me check if I can fix this.

@MorrisJobke
Copy link
Member

MorrisJobke commented Feb 2, 2018

But for now this is good to get in 👍

@MorrisJobke
Copy link
Member

MorrisJobke commented Feb 2, 2018 •
edited
Loading

Maybe also add a little spinner once the button is pressed to give immediate feedback - or disable the button for that time.

@nickvergessen
Add an endpoint to delete all notifications
5ce598d 
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
Copy link
Member Author

nickvergessen commented Feb 2, 2018 •
edited
Loading

Maybe also add a little spinner once the button is pressed to give immediate feedback - or disable the button for that time.

Also for actions and the normal dismiss, so lets do that in a new issue: #115

@nickvergessen
Add a "Dismiss all notifications" button at the end
6af4380 
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen force-pushed the feature/105/dismiss-all-button branch from 9429570 to 6af4380 Compare February 2, 2018 13:04
@nickvergessen
Copy link
Member Author

nickvergessen commented Feb 2, 2018

Rebased because of #114

@nickvergessen nickvergessen merged commit be5504f into master Feb 2, 2018
@nickvergessen nickvergessen deleted the feature/105/dismiss-all-button branch February 2, 2018 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MorrisJobke MorrisJobke MorrisJobke approved these changes

Assignees

No one assigned

Labels

3. to review enhancement feature: backend 🛠️ feature: frontend 🖌️

Projects

None yet

Milestone

Nextcloud 14

Development

Successfully merging this pull request may close these issues.

3 participants

@nickvergessen @MorrisJobke