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

Atrules update - Purge @font-face, Small refactor #48

Merged
merged 6 commits into from
Jan 30, 2018
Merged

Conversation

jsnanigans
Copy link
Collaborator

Proposed changes

Only use one .walk to do everything,

  • check if its a normal rule or a atRule - line 250
  • if its a normal rule then run the existing check, evaluateRule and if the rule is not removed then record the relevant declarations - line 309 to 322
  • if its a atRule then register it for later regerence - line 256

after that there are two simple for loops that remove the keyframes and the font-faces, line 150 and 164

There might be a bug if someone uses the css font: 'My Fam' 20px because I only check for font-family: 'My Fam' because im not sure how to handle it if there are quotes around the name.

also #41 is fixed

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@Ffloriel Ffloriel self-requested a review January 29, 2018 21:40
Copy link
Member

@Ffloriel Ffloriel left a comment

Choose a reason for hiding this comment

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

Great PR! Thanks a lot.
Looks good to me. I just think it is better to keep the options in camelCase.

@jsnanigans
Copy link
Collaborator Author

Sure, did the changes

@jsnanigans jsnanigans closed this Jan 30, 2018
@jsnanigans jsnanigans reopened this Jan 30, 2018
@Ffloriel Ffloriel merged commit 3c5bb94 into master Jan 30, 2018
@Ffloriel Ffloriel deleted the atrules-update branch May 16, 2018 21:50
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.

2 participants