search

67P / hyperchannel

Kosmos Chat for the Web
Mozilla Public License 2.0
20 stars 3 forks source link

Add a class to ops in the users list #106

Closed gregkare closed 7 years ago

gregkare commented 7 years ago

Refs #21

It works but I haven't added styling yet. This should also fix messaging ops from the user list

raucao commented 7 years ago

I increased the mass threshold for the CodeClimate duplication warnings. These should be green in the future, unless you have actual duplication going on.

gregkare commented 7 years ago

I'm going to remove the User model for now, switch back the user list to an array of strings and use a helper to render each user list. The sort order works automatically because of the prefixes

gregkare commented 7 years ago

This is ready to review. I haven't changed the UI for it, but this adds a "normal" class to normal users and "op" to ops

gregkare commented 7 years ago

+ comes before @ when doing a .sort(), so we'll need a custom sort function

raucao commented 7 years ago

Sounds like a seperate new issue to me, because they're already sorted by that first character.

gregkare commented 7 years ago

There's still an issue with this PR: there's a double rendering issue.

@galfert Do you know what I should do to fix it?

"Assertion Failed: You modified "unreadMessagesClass" twice on <(unknown mixin):ember1638> in a single render. It was rendered in "component:channel-nav" and modified in "component:link-to". This was unreliable and slow in Ember 1.x and is no longer supported. See https://github.com/emberjs/ember.js/issues/13948 for more details.
EmberError@http://localhost:4200/assets/vendor.js:22898:19

This seems to happen when joining a new channel, and goes away when reloading Hyperchannel

raucao commented 7 years ago

@galfert Did you see @gregkare's question here?

galfert commented 7 years ago

No, I missed that. Might be some runloop issue. I'll have a look.

galfert commented 7 years ago

When testing this I had the (already known and fixed) errors due to the incompatibility with the recent Sockethub changes. So I merged in the current master. But I still couldn't reproduce the error you mentioned.

The feature itself worked. But I took the liberty to refactor it a little bit to be less "if-elsy" :) I also added some unit tests.

Overall I think it's good to merge now.