bengott/meteor-avatar

Please don't change Avatar sizes

Closed this issue · 6 comments

I've just pulled the latest version and my site design it now broken because you changed the size of the standard avatar from 40px to 50px.

Either this need to be fixed in stone, or else user controllable.

I'm sorry your site got broken because of the recent changes, but that's life on the bleeding edge of pre-1.0 software! ;) Unfortunately, it's not as easy as making size a user-defined number like this: {{> avatar size=40}} because the CSS classes can't be dynamically defined like that. See the comments on issues #3 and #14 for more explanation. Fortunately, it's very easy to override the default styles if you want. Just add the following to your CSS (Stylus) and you're standard avatars will be back at 40px.

.avatar
  height 40px
  width 40px
  .avatar-initials
    font-size 20px
    line-height 40px

The css changes are ok, but that doesn't stop the image elements receiving the wrong dimensions.

Rather than having 'small', 'large', etc., how about creating size names like '10px', '20px' up to '100px'? That'll give us more flexibility and it's unambiguous.

Yeah, you're right about the image width and height attributes, and that adds a wrinkle to overriding sizes in the CSS. Your size names idea is an option -- please submit a PR if you want to. The only other option for totally user-defined sizes is to inject inline styles like the bgColor and textColor params do, but I don't think we want to do that. It's OK for the occasional one-off override, but not as a general rule. I'm actually thinking about removing those params altogether...

As far as the image dimensions, I think maybe it makes sense to break it out into another parameter in order to override the defaults if you provide your own styles. Then your template call would look like this: {{> avatar user=this imageSize=40}} or {{> avatar user=this size='large' imageSize=100}} or maybe even {{> avatar user=this class='extra-large' imageSize=120}} with extra-large as a custom CSS class. Or does all that sound too complicated? I welcome your (and anybody else's) input on this. @mquandalle, @Kestanous, @kevohagan, @nickw, @SachaG: care to comment?

Another option I just thought of is to make it a config option, that way you wouldn't need to remember to do it on every template call. You would just define it once along with the CSS rules and then forget about it:

Avatar.options = {
  imageSizes: {
    large: 96,
    medium: 48, //default
    small: 24
  }
}

You'd still need the template parameter for any custom classes, but at least it would allow for custom overrides of the defaults.

Yeah, I would say it's best not to change the sizes from now on. But in any case, most people will probably want to set the size through their own styles. So I think my ideal solution would be not setting a width at all by default (maybe just a max-width to avoid huge avatars).

I also like having the size in the config though, that seems pretty handy.

I've just issued a pull request to solve this issue. Using my update it's now possible to do something like this:

Avatar.setImageSizes({
  'large': 90,
  'mySize': 40
});

Which will override the default large size and create a new mySize one. The CSS in injected into the DOM, and the IMG element receives the current height and width.

Note that, per your previous comment, the style is injected as a new style just once into the DOM, and not for every element as you're doing for the background and text colour.