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

feat(ligatures): adds a proposal to add ligatures #315

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

oscargm
Copy link

@oscargm oscargm commented Nov 24, 2021

Related with #182

@vfreitas-
Copy link

@oscargm I think its probably better to revert the changes on the package-lock file? As they seem not necessary for this change (probably its only a npm version difference) it would help the @tancredi review.

@tancredi You think this is a good change? If you need more context, this would allow the font to be used with the getIconId text as an inline text of the html element instead of using the unicode for example:

Json generated with fantasticon

{
  "bank-fill": 61697,
  "bank": 61698,
  "building-fill": 61699,
  "building": 61700,
  "community-fill": 61701,
  "community": 61702,
  "government-fill": 61703,
  "government": 61704
}
.my-icon {
  font-family: icons; // font family generated with fantasticon
  font-weight: normal;
  font-style: normal;
  font-size: 24px;
  display: inline-block;
  line-height: 1;
  text-transform: none;
  letter-spacing: normal;
  word-wrap: normal;
  white-space: nowrap;
  direction: ltr;

  -webkit-font-smoothing: antialiased;
  text-rendering: optimizeLegibility;
  -moz-osx-font-smoothing: grayscale;

  font-feature-settings: 'liga';
}
<span class="my-icon">building</span>

The html would render the icon without using the before or after pseudo elements.

@oscargm
Copy link
Author

oscargm commented Jan 5, 2022

Yeah sure @vfreitas- no problem.

I was awaiting for @tancredi's feedback, if he agrees with the proposal I can try to continue and make a proposal for extending the configuration and fix the tests.

@jamesmsk
Copy link

waiting for this PR too!!!!!!

@oscargm
Copy link
Author

oscargm commented Mar 4, 2022

Fixing snapshot testing here

@oscargm
Copy link
Author

oscargm commented Mar 4, 2022

Also added a field named addLigatures in configuration and modified test accordingly

@oscargm
Copy link
Author

oscargm commented Mar 18, 2022

@tancredi could we review the PR and merge it in case it's okay?

@lampewebdev
Copy link

Waiting for this too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants