Skip to content

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

Merged
Ffloriel merged 6 commits into
masterfrom
atrules-update
Jan 30, 2018
Merged

Atrules update - Purge @font-face, Small refactor#48
Ffloriel merged 6 commits into
masterfrom
atrules-update

Conversation

@jsnanigans

Copy link
Copy Markdown
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

@Ffloriel Ffloriel left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
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