FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add Body Class and Ignore Class by davidhellmann · Pull Request #143 · feimosi/baguetteBox.js · GitHub

Add Body Class and Ignore Class - #143

Merged
feimosi merged 3 commits into
feimosi:devfrom
davidhellmann:dev
Sep 15, 2017
Merged

Add Body Class and Ignore Class#143
feimosi merged 3 commits into
feimosi:devfrom
davidhellmann:dev

Conversation

Copy link
Copy Markdown
Contributor

Hey, here is a new Pull Request. Now you can see it better I hope :)
Sorry for the late response!

feimosi commented Jan 31, 2017

Copy link
Copy Markdown
Owner

No problem, thanks! There are a few linter errors, could you take a look?
https://travis-ci.org/feimosi/baguetteBox.js/jobs/195095908#L188

Copy link
Copy Markdown
Contributor

@feimosi: just fix them yourself and push to @davidhellmann fork and then just squash + merge.

feimosi commented Sep 15, 2017

Copy link
Copy Markdown
Owner

I can't push to other's forks without some extra permission, can I?

XhmikosR commented Sep 15, 2017 via email

Copy link
Copy Markdown
Contributor

feimosi commented Sep 15, 2017

Copy link
Copy Markdown
Owner

@XhmikosR Interesting, thanks! :)

feimosi merged commit f3a6cfc into feimosi:dev Sep 15, 2017

Copy link
Copy Markdown
Contributor

@feimosi: I replied via email but you probably missed it.

Now regarding this PR... are you sure all supported browsers have classList support?
We should add ESLint plugin compat to automate this.

feimosi commented Sep 16, 2017

Copy link
Copy Markdown
Owner

@XhmikosR I didn't get that message.
Anyway, I've added an extra check for classList support here:
https://github.com/feimosi/baguetteBox.js/pull/143/files#diff-4634650f8cf2cdceb34291a9bcf4c23fR472

I'll test test thoroughly before the next release too.

Copy link
Copy Markdown
Contributor

@feimosi: I wouldn't personally go with that. If you still want to support < IE 9 you should do this https://plainjs.com/javascript/attributes/adding-removing-and-testing-for-classes-9/

feimosi commented Sep 16, 2017

Copy link
Copy Markdown
Owner

Sure, it works only in newer IE, but using this option won't break anything, so I don't see any hazard here. Treat it as progressive enhancement, I just need to mention that in the docs.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

3 participants


Back | FazBrowse Home | New Git URL