Added class property to Vue.use() options

[JGJP]

Made it possible to specify a class to be added to all elements that have the v-scroll-reveal directive on them. I've found this to be useful for nullifying the reveal effects from css with media queries or when performing visual regression tests.

[tserkov]

The only issue I have with this is scope; I feel this type of change would be better served being made within ScrollReveal itself, rather than the Vue wrapper for it. I'm sure if you were to make the suggesting or pull request to @jlmake, he may be willing to add it to the library, which we're merely making more readily usable in Vue.

[JGJP]

I see what you're saying. The reason I think this is applicable to the wrapper and not the original is that ScrollReveal uses classes already, so all elements are easily selected from CSS. With vue-scroll-reveal you'd have to add a class to each element in addition to the directive, which feels redundant, IMO.

[tserkov]

ScrollReveal actually sets style attributes directly, no classes (which is by design, and also to the chagrin of those who would like to use animate.css).

I've actually just discovered jlmakes/ScrollReveal#394, which talks about exactly this; I would just need to upgrade the version of ScrollReveal used by this wrapper (currently v3.3.6) to v4 (currently in beta).

[JGJP]

I meant that ScrollReveal uses classes to decide what elements to set style attributes to:

sr.reveal('.foo');

in this situation, it's easy to set a rule like this:

@media print {
  .foo{
    opacity: 1!important
    transform: translateY(0)!important
    transition: none!important
  }
}

As it stands now, with vue-scroll-reveal you'd have to add classes all over your markup, in addition to the directive, since the directive isn't selectable from CSS:

<section class="foo" v-scroll-reveal.reset>
  <h1>Tada!</h1>
</section>

<section class="foo" v-scroll-reveal.reset={ delay: 250 }>
  <h1>Slightly late tada!</h1>
</section>

What I'm proposing would allow this:

Vue.use(VueScrollReveal, {
  class: 'foo'
});

<section v-scroll-reveal.reset>
  <h1>Tada!</h1>
</section>

<section v-scroll-reveal.reset={ delay: 250 }>
  <h1>Slightly late tada!</h1>
</section>

This rule would still work, without dirtying up any template markup:

@media print {
  .foo{
    opacity: 1!important
    transform: translateY(0)!important
    transition: none!important
  }
}

(I think one might be surprised how many people there are that print webpages, try to print https://scrollrevealjs.org/)

[tserkov]

Ah, I misunderstood. I can definitely see the use case then, since this is a directive vs. the traditional usage of class selectors.

I'm a staunch believer in not placing markup (ie. .foo) in application logic (ie. main.js), and explicit v. implicit; however, I'm also a believer in not imposing my design principles upon others for a library, so -- let's get this merged!

I made some comments on the commits for some basic style changes to keep the codebase consistent with AirBNB's format guide. Once those changes are made, I'll perform the merge.

Thanks.

[JGJP]

Great! I guess I should have been clearer with my initial explanation, my bad.

I'm happy to make any style changes to the code. Please forgive my ignorance if I'm overlooking something but where should I be looking to see your comments? I'm not seeing anything on the commits in my branch.

Cheers.

[tserkov]

I'm able to see the review comments inline this thread, and the Files Changed tab at the top.

[JGJP]

I think maybe you need to complete the review process and request changes before I can see them? I see nothing in this thread or in the Files Changed list.

[tserkov]

All merged; thanks for the excellent contrib!

[JGJP]

Always a pleasure!