mobify / stencil

DEPRECIATED - The latest Stencil development is currently taking place in the Adaptive.js repo.
MIT License
4 stars 0 forks source link

Badge Component #111

Closed avelinet closed 9 years ago

avelinet commented 9 years ago

Status: Ready to merge

Reviewers: @jeffkamo @kpeatt @cole-sanderson @nastiatikk @mlworks

Minor work todo

kpeatt commented 9 years ago

I did two things:

I think we should add modifiers to this for things like success, error, warning, and whatnot. Might want to wrap the content in a span so we can separate text or content styling from the badge styling.

ry5n commented 9 years ago

In general this looks really good. My main concerns are:

kpeatt commented 9 years ago

@ry5n I think we probably want the badge to move to a pill shape when the font size gets wider but that's a design preference.

ry5n commented 9 years ago

@kpeatt I think that’s sensible default behaviour. What I’m saying is that it should not get narrower if you use a condensed face though. So it should be circular, or grow horizontally. Hence setting height and min-width.

kpeatt commented 9 years ago

@ry5n :+1:

avelinet commented 9 years ago

I agree with omitting the modifiers for the base component, can't think of the last project I would have needed them on a badge component.

Also agree with @ry5n regarding wrapping the inner text.

ry5n commented 9 years ago

+1

ry5n commented 9 years ago

@kpeatt Any comments on the last two todos in the summary? Do you disagree?

kpeatt commented 9 years ago

Nope, I don't think they're blockers and we could add them later if necessary.

kpeatt commented 9 years ago

:+1: from me

ry5n commented 9 years ago

Implementation looks good. It’s smaller than it was; what do you think of bumping the font-size to 0.85em?