Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

feat(icons): add support for font-icons using ligatures #3059

Merged
merged 1 commit into from
Jun 1, 2015

Conversation

ThomasBurleson
Copy link
Contributor

Support for md-font-library="" attribute
Improved documentation
Extra demo using Material Design icons.

@ThomasBurleson ThomasBurleson added this to the 0.10.0 milestone Jun 1, 2015
@ThomasBurleson
Copy link
Contributor Author

@robertmesserle, @jelbourn - ready for your review.

var names = attrValue('mdFontLibrary') + ' ' + attrValue('class');
element.attr('class',names.trim());

return $interpolate( tmpl )({ classNames: names.trim() });
Copy link
Contributor

Choose a reason for hiding this comment

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

Using $interpolate here seems a bit like overkill - why not just include names.trim() in the original template string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At one point, the tokenized requirements were more complicated. I will adjust L151.

@robertmesserle
Copy link
Contributor

@ThomasBurleson LGTM, just made some minor comments.

@ThomasBurleson ThomasBurleson merged commit a107490 into master Jun 1, 2015
<div ng-controller="DemoCtrl" ng-cloak >

<p>
Display 4 Material Design font-icons using ligatures [instead of CSS names]; each with different sizes and colors<br/>
Copy link
Member

Choose a reason for hiding this comment

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

Why does a <p> end with a <br>?

@MigFerreira
Copy link

Hi,

I cant seem to get the icon visible inside a button.

codepen: http://codepen.io/MigFerreira/pen/RPpapj

@shyndman
Copy link
Contributor

shyndman commented Jun 2, 2015

Looks like letter-spacing: 0.1em on .md-button kills the ligatures.

@ThomasBurleson I'm going to ask the Fonts team to make a change to the served CSS. Might take a day or two.

@shyndman
Copy link
Contributor

shyndman commented Jun 2, 2015

OK, Fonts team just submitted a CL with the fix. I've asked them to expedite a push to prod.

@shyndman
Copy link
Contributor

shyndman commented Jun 3, 2015

@MigFerreira Try your demo again. Should work properly.

@MigFerreira
Copy link

@shyndman Thanks for fast action, looking great!

@ThomasBurleson
Copy link
Contributor Author

Thx @shyndman.
@marcysutton - do you recall why we have this in typography.scss:

.md-button {
  letter-spacing: 0.010em;
}

@marcysutton
Copy link
Contributor

@ThomasBurleson it was for the "tracking" section in the Typography spec (aka letter-spacing): https://www.google.com/design/spec/style/typography.html#typography-other-typographic-guidelines

@ThomasBurleson ThomasBurleson deleted the wip/font-icons-ligatures branch January 19, 2016 04:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants