Skip to content
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

Add tagline support for brand icon #6

Merged
merged 4 commits into from
May 4, 2017
Merged

Conversation

thibautRe
Copy link
Contributor

screen shot 2017-04-13 at 14 01 56

Overview

Add support for tagline under the Fyndiq Logo.

How to test

  • npm run dev and go to localhost:6006
  • Check the Icon Brand -- with tagline and see how it behaves

Copy link
Contributor

@simonmalmgren simonmalmgren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work! Just one small detail regarding fontsize that could be fixed.

.tagline {
fill: @red;
text-anchor: middle;
font-size: 6.5px;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having it at 5.5px makes it look more like the logo we display on the monolith (desktop)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took for reference the mobile website haha.

Since the 2 websites are already not consistent, I can add a prop to the component, like taglineSize which would be the size in px. And default would be either 6.5px or 5.5

This enables the use to chose the font-size of the tagline
@thibautRe
Copy link
Contributor Author

@simonolsson ping

@thibautRe thibautRe merged commit 0412ebf into master May 4, 2017
@thibautRe thibautRe deleted the tagline_brand_icon branch May 4, 2017 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants