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

detect if the instance created extends or direct instance of Smooth #17

Closed
baptistebriel opened this issue Feb 7, 2016 · 11 comments
Closed

Comments

@baptistebriel
Copy link
Owner

Currently to extend Smooth in your own file you have to pass an option. I need to know if the instance created extends Smooth or is direct instance of Smooth.

https://github.com/baptistebriel/smooth-scrolling/blob/master/index.js#L17

I've done this so smooth-scrolling.js could be used on its own without going into the build process.

@Awea
Copy link

Awea commented Sep 26, 2016

@baptistebriel something like this this.extends = this.constructor.name != 'Smooth' in the constructor ?

@baptistebriel
Copy link
Owner Author

Awesome, thanks for the hint @Awea ! 👍

baptistebriel pushed a commit that referenced this issue Sep 26, 2016
@Awea
Copy link

Awea commented Sep 26, 2016

@baptistebriel np :) Now I wish I can do what I want with this library !

@baptistebriel
Copy link
Owner Author

Are you having issues with it? Don't hesitate to ask 🙂

@Awea
Copy link

Awea commented Sep 26, 2016

If I get stuck at some point yeah, thanks !

@jghlt
Copy link

jghlt commented Jan 11, 2017

Hi
great library thanks!

When I extend the Class, vars.bounding is never set.

index.js line 288

if(!this.extends) {
  const bounding = this.dom.section.getBoundingClientRect()
  this.vars.bounding = this.vars.direction === 'vertical' ? bounding.height - (this.vars.native ? 0 : this.vars.height) : bounding.right - (this.vars.native ? 0 : this.vars.width)
}

Is this desired behaviour?

@Awea
Copy link

Awea commented Jan 12, 2017

Hi @jghlt,

The vars.boundings is correctly set using one of the demo. Can you please provide an example where this is not the case ? :)

@baptistebriel
Copy link
Owner Author

Hello @jghlt,

Yes indeed, this is an expected behaviour; when you use an extended version of Smooth, you might want to set this.vars.bounding manually because it could be something else rather than the height of this.dom.section. I did this so you could have more control on the scrollable content, while being able to use smooth-scrolling.min.js without extending it.

Let me know what you think!

@jghlt
Copy link

jghlt commented Jan 12, 2017

@Awea in the demos it seems to be explicitly set again within the custom.js for each demo.

@baptistebriel Cool, its not a big deal to have to set this again, but might be nice to have an option to retain the default functionality, unless I'm thinking about this in the wrong way?

@baptistebriel
Copy link
Owner Author

We could check something like this:

if(!this.extends || this.options.keepDefaultBounding) {
  const bounding = this.dom.section.getBoundingClientRect()
  this.vars.bounding = this.vars.direction === 'vertical' ? bounding.height - (this.vars.native ? 0 : this.vars.height) : bounding.right - (this.vars.native ? 0 : this.vars.width)
}

... but it adds another option, and I thought it would be just simpler to have complete control on the amount of pixels it can scrolls when it's a custom class of smooth-scrolling.

@jghlt
Copy link

jghlt commented Jan 12, 2017

I agree its better to keep options to a minimum, but I think it would be good to have the option to maintain the default behaviour in this instance, what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants