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

[WIP] Frontend implementation for notifications #439

Merged
merged 80 commits into from Apr 18, 2019

Conversation

roschaefer
Copy link
Contributor

@roschaefer roschaefer commented Apr 8, 2019

Pullrequest

Issues

Checklist

  • None

How2Test

  • None

Todo

  • Shall we re-use PostCard.vue in Notification.vue?
  • Write a cucumber test to mark a notification as read

@mattwr18
Copy link
Member

on the root path /
Screenshot at 2019-04-09 21:25:16
on a post /post/<id>/<slug>
Screenshot at 2019-04-09 21:27:58
on a user's profile, it looks the same as on the root path

@ulfgebhardt ulfgebhardt added this to the Sprint: Hera milestone Apr 10, 2019
@ulfgebhardt ulfgebhardt added this to To Do in Human-Connection via automation Apr 10, 2019
roschaefer and others added 15 commits April 17, 2019 00:08
Note that we don't create duplicate notifications. I made use of the behaviour
of XSS-middleware: It removes all css classes from `<a>` anchors. Because
notifications rely on a css class `mention` which gets removed in a later
middleware, this gives us a nice behaviour for re-notifications without creating
duplicates. The downside is that it creates dependencies between middlewares and
it's not that obvious at all.

cc @mattwr18 @ulfgebhardt @appinteractive @Tirokk
This will help some people not to loose data after accidently clicking
on the user @-mentioning.
Fixed a bug along the way `(post.contentExcerpt == null)`
cc @mattwr18 Please don't see this as a revert of your work. Your
structure of the `notification-post-card` component was helpful and showed
the redundancy with `hc-post-card`. I reused a lot of the code, but
because I merged both components it now *looks* as if I authored all the code.
Unfortunately with `v-html` you cannot use filters directly in
handlebars.
See: nuxt/nuxt#231

I also fixed the tests even **without** mocking vue-filters.js plugin 👍
…on/Human-Connection into 347-display_notifications
@roschaefer roschaefer changed the title Frontend implementation for notifications [WIP] Frontend implementation for notifications Apr 17, 2019
@roschaefer roschaefer force-pushed the 347-display_notifications branch 2 times, most recently from 40c7578 to 57a53a4 Compare April 18, 2019 14:41
cc @mattwr18 @Tirokk @ulfgebhardt @appinteractive

The factories don't use string interpolation now but they use variables.
This resolves some errors of escaping strings when you send html along
with `post.content`. It is much cleaner, too.
Sometimes, the popup was covering the create-post button thus failing
some cypress tests locally.
@roschaefer roschaefer merged commit 6cd8a4e into master Apr 18, 2019
Human-Connection automation moved this from To Do to Done Apr 18, 2019
@roschaefer roschaefer deleted the 347-display_notifications branch April 18, 2019 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

Click on notification and mark it as read Display notifications in the frontend
4 participants