FEATURE: Publish read state on group messages. (PR #7989)

GitHub

You’ve signed the CLA, romanrizzi. Thank you! This pull request is ready for review.

Might be wrong but I think

document.querySelector(`.indicator-topic-${data.topic_id}`)

would do the same

Is there any reason

if (this.get("topic.read_by_group_member")) {}

wouldn’t work ?

We want to be subscribed even if topic.read_by_group_member is false, that’s why we explicitly check for undefined.

Thanks, @jjaffeux <3

I don’t see an equivalent unsubscribe. This should probably be in a didInsertElement and the unsubscribe should be in the willDestroyElement

1 Like

I think if you don’t want an icon you should be able to leave this attribute out? If that doesn’t work we should adjust the button code to handle undefined instead of an empty string

1 Like

Same comment as above but for className here - we should be able to omit it.

1 Like

In what case would the object not have this method?

1 Like

I Should have added a comment here. This attribute is dynamically calculated here.

You should have a this._super(...arguments) in here and willDestroyElements just to be safe.

1 Like

I see, that should be OK although traditionally respond_to? is slow. We probably won’t notice it.