Skip to content
This repository has been archived by the owner on Feb 12, 2021. It is now read-only.

Create Swiper module and component #390

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

fstgerm
Copy link

@fstgerm fstgerm commented Apr 17, 2020

No description provided.

@fstgerm fstgerm marked this pull request as draft April 17, 2020 15:59
@fstgerm fstgerm marked this pull request as ready for review April 17, 2020 16:00
src/modules/auto-swiper.js Outdated Show resolved Hide resolved
}
};

var DATA_KEY = 'swpr';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use swiper as a key ? or this is already taken by the lib ?


return {
init: init,
reset: reset
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should include a destroy method

return config;
};

var init = function (element) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our init methods always need a second params to add/override the default config of the swiper instence

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See flickity.js for reference

return obj;
};

var getConfig = function (element, prefix) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the fx computeOptions.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't support nested options.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, will update computeOptions to support this. Still use it tho.


'use strict';

App.components.exports('swiper', function (s) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be able to override default config too via s

Copy link
Contributor

@fhamon fhamon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments above.

Comment on lines +31 to +38
var nestedConfigs = [
'navigation',
'scrollbar',
'pagination',
'thumbs',
'autoplay',
'keyboard'
];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think that we should add all the Nested Configs available ? ex : mousewheel

If you think so, there's more that we can add ;)

Thank you

Copy link

@SamStamkos SamStamkos Apr 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should define which one is added by default :)

@nitriques @fhamon @emmacharp @dominic-blain @fstgerm @alexnantel88 @f-elix @beaubbe

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

Successfully merging this pull request may close these issues.

3 participants