#7469 - stop ignoring from hovercards

[lu-x]

No description provided.

[lu-x]

@Flaburgan I pushed my first try. Tests are still missing. I'll add them later.

[diaspora-code-review]

Strings must use doublequote.

[lu-x]

I updated the specs. Ready for some reviewing.

[SuperTux88]

Thanks for your PR, I tested it locally, but I found a problem. In some cases the hovercard doesn't reload properly (see comment below), also it would be cool to have some more tests for this.

[SuperTux88]

There is a race-condition with this. The unblock happens in parallel with the reload, so when the reload reloads the hovercard.json before the unblock is persisted, the hovercard is stuck in an inconsistent state (happens in about 50% of my tries):

There is an event triggered when the unblock is done, see:

diaspora/app/assets/javascripts/app/pages/profile.js

Line 39 in b4e3f84

app.events.on("person:unblock:"+id, this.reload, this);

Maybe you can do it similar?

[lu-x]

I tried something like app.events.on("person:unblock:" + id, this.parent.reload, this); in the unblock person view. But somehow the event action always triggers on initialization and therefore caused an endless loop.
I now do the refresh, if the unblocking is done.

[SuperTux88]

Thanks for the update and sorry for the slow response.

I tried something like app.events.on("person:unblock:" + id, this.parent.reload, this); in the unblock person view.

It works for me with app.events.on("person:unblock:" + this.person.id, this.parent._populateHovercard, this.parent);, however the event is added everytime the hovercard opens, so when you open/close it multiple times before clicking the button, the hovercard is reloaded multiple times afterwards (not visible, but there are multiple requests visible in the dev-tools). So the event should be removed again when the hovercard is closed, so there is only one event.

However your current solution seems to work too, but I don't know which is the cleaner solution. I'm not a frontend guy and this is becoming to much javascript for my knowledge. Hopefully somebody with more JS knowhow can review the PR and tell if the current solution is OK to merge.

See above.

[HankG]

Now that this method is called outside of the class it should probably drop the leading underscore since it is no longer private use. The method itself assumes that parent has been set with a parent that has the href attr which is only true if the parent used already went through the showHovercardOn: _.debounce path. It seems any time someone would use it that it would have been through that path already but if want to do some more defensive programming could catch/check that too perhaps.

[HankG]

Not a core member but think this looks pretty good. It works well in the browser in testing on my instance too. I had a comment on the formerly "private" member. I know the name should change but not sure if we need to cover case where it is invoked before the card is shown (which triggers the event that populates the parent of the hovercard).

[denschub]

See previous comment.