-
-
Notifications
You must be signed in to change notification settings - Fork 765
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
Display occupants avatar #1354
Display occupants avatar #1354
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for contributing. I've left some comments after reviewing your PR.
'image': "data:" + image_type + ";base64," + image, | ||
}); | ||
}, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no need to duplicate the code. Instead, in src/converse-chatboxviews.js
, you can set AvatarMixin
as an attribute on the _converse
object.
Then further down you can do:
_converse.ViewWithAvatar = Backbone.NativeView.extend(_converse.AvatarMixin);
sass/_roster.scss
Outdated
@@ -119,7 +119,9 @@ | |||
|
|||
.avatar { | |||
float: left; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're using flexbox for many things now, so this float: left
statement might no longer be necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. In fact, I found it here, and when I tried to remove it, it broke the design. I'll try to remove it again and fix the display with flexbox.
This fixes #1322, btw. |
8ca2731
to
f16b6d2
Compare
d9824b4
to
c403708
Compare
04f88d1
to
3e4f4e9
Compare
37cfa36
to
f1899d0
Compare
aa8c887
to
d7d810b
Compare
I've made some updates to this PR and then did a manual merge. |
Hi,
this is a first contribution. There are some
I copied the AvatarMixin from chatbox-views, would have it been better to import the whole file, or maybe abstract this mixin in separate file ?