search

richaber / twistermc-bb-modules

0 stars 1 forks source link

Questions / Discussion / Direction #1

Open richaber opened 6 years ago

richaber commented 6 years ago

Looking at the Todos in the Readme one by one...

I added a conditional check in the Slick module that only enqueues the Vimeo and/or YouTube scripts if we have Vimeo and/or YouTube embed slides.

Since Slick itself has no concept of "Autoplay Videos," or "Show Image Captions," or "Force images to full width" I moved those settings to a new tab called "Media Settings." I feel that anything that is not specific to Slick's settings should be kept separate from the other slider settings to reduce confusion. This may not be a hard and fast rule, it might be a little fuzzy due to settings like "Design."

I'd need to know what the markup output should look like. And nobody wants me styling that.

Needs more info.

Done. Uses a "Settings Form" field.

I'd need to know what the markup output should look like. And nobody wants me styling that.

I'm not sure I understand that exactly, but I'm using the Vimeo Player API (which replaced the deprecated Froogaloop library) loaded from Vimeo's own CDN.

And then there's this one item from the change log, not listed as a Todo...

That included an old version of Slick (v 1.6.0) with an unofficial patch. According to the Slick 1.7.1 release notes this should be addressed in newer releases:

Fixed Case of white space with variable width and infinite true

Since I'm not a fan of using outdated libraries, I've opted to use the latest official Slick release (1.8.1 as of this writing).

richaber commented 6 years ago

@TwisterMc can you have a look at what I've got for open questions / comments / whatever here and see what you think? I can submit a PR with what I have right now, if you prefer, but some of this should maybe hashed out a little first?