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

AMP support #57

Closed
fstanis opened this issue Aug 31, 2018 · 4 comments
Closed

AMP support #57

fstanis opened this issue Aug 31, 2018 · 4 comments

Comments

@fstanis
Copy link
Contributor

fstanis commented Aug 31, 2018

I'm experimenting with using PostHTML and htmlnano with a page I've built using AMP and, while it works fine right now (as AMP is just HTML), I think there are some minor tweaks to htmlnano that would make it even more useful.

Notably:

  • AMP has some additional attributes that could be added to collapseBooleanAttributes, such as amp, amp-custom, amp-boilerplate.
  • mergeStyles and minifyCss should ignore the AMP boilerplate (<style amp-boilerplate>...</style>).
  • amp-custom should be preserved on the merged <style> tags, if present (maybe just include this in styleKey).
  • There may be additional attributes to be added to removeRedundantAttributes and removeEmptyAttributes (still looking into this).

I'm happy to send a PR with these changes, but wanted to start some discussion first. Would you be OK with accepting these additions to htmlnano?

@maltsev
Copy link
Member

maltsev commented Aug 31, 2018

Would you be OK with accepting these additions to htmlnano?

Totally! I'll appreaciate it if you implement these minifications.

@fstanis
Copy link
Contributor Author

fstanis commented Sep 3, 2018

#59 adds some of the things mentioned. I'll still need to look into whether additions can be made to removeRedundantAttributes and removeEmptyAttributes, but this should make htmlnano a lot more useful for AMP pages already.

@maltsev
Copy link
Member

maltsev commented Sep 4, 2018

Great!

@fstanis are you going to submit new PRs this week (except #60)? If yes then I'll wait for your new PRs before releasing a new htmlnano version. Otherwise I'll release it as soon as #60 gets merged.

@fstanis
Copy link
Contributor Author

fstanis commented Jan 3, 2019

I think I'll close this for now. It seems AMP support is in a good place now based on my testing - if we missed something, it's easy to reopen.

@fstanis fstanis closed this as completed Jan 3, 2019
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

No branches or pull requests

2 participants