search

ArthurClemens / mithril-slider

Content slider for Mithril for mobile and desktop
30 stars 5 forks source link

Function like a carousel? #5

Closed JohnPaulHarold closed 7 years ago

JohnPaulHarold commented 7 years ago

Is it possible for this slider component to be configured to operate like a carousel? Meaning, with n amount of previous and next slides/pages either side of the centre aligned current slide/page.

Example, https://owlcarousel2.github.io/OwlCarousel2/demos/basic.html

TimMensch commented 7 years ago

Ha! Funny you should ask.

I just spent a bunch of time making my fork work like a carousel:

TimMensch/mithril-slider

I just hacked together some quick docs. But I haven't been able to build the ES5 JavaScript. Doesn't work for me to run npm run transpile. Yes I have babel installed. It says:

es6 file found, transforming lib\mithril-slider-style.es6
es6 file found, transforming lib\mithril-slider.es6

...which then does nothing. Homemade build scripts for the loss. :(

But I added es:next to the package.json, and I'm importing the code as ES6 in my main project, so I don't actually need the ES5 build to work myself. In fact for me it's better this way, because I can debug with full source, and then only uglify when I'm making a production build.

TimMensch commented 7 years ago

If you do get a transpile to work, go ahead and send a pull request and I'll merge it.

ArthurClemens commented 7 years ago

PRs are welcome!

TimMensch commented 7 years ago

@ArthurClemens I would, but I'm crunching on an internal deadline right now, and I've done zero testing outside of my own use case (Promises, not m.request, as mentioned in the issue I filed). So I know the code Works For Me with Promises, but have no idea if it still works on m.request().

Also, if you see above, I can't even build the ES5 version -- something about the build scripts are broken for me. Maybe a missing dependency with a silent failure? Maybe the build scripts quietly choke on Windows paths? Seems like a bad idea to send a pull request that has new ES6 and old ES5.

If I manage to get a breather, I'll look into tracking down why the build scripts fail. But this is why it's a good idea to use gulp-based build scripts or WebPack: They're easier to use deterministically, and generally Just Work™ on Mac, Linux, and Windows.

If you want me to send a pull request for all the changes I made, I can do that, but I'd do that with a giant caveat that it needs more testing and verification. And to have the ES5 code built.

ArthurClemens commented 7 years ago

This is an old build script. I am revisiting my Mithril projects to update this, along with rewriting for Mithril 1.0. Perhaps better to revisit this after the update.

TimMensch commented 7 years ago

I'm stuck on 0.2.5 for the time being, but at some point I'll probably want to upgrade as well. Again, though, my current priority is getting the MVP for my project working.

JohnPaulHarold commented 7 years ago

@TimMensch Thanks, will take a look.

ArthurClemens commented 7 years ago

An example will be included in the upcoming version.

JohnPaulHarold commented 7 years ago

@ArthurClemens are you planning a similar v1 update for this module also?

ArthurClemens commented 7 years ago

Yes, but I am fixing the Promise bug and cleaning up the code first.

ArthurClemens commented 7 years ago

Check out the "Centered image" example in release 0.4.0, live version at http://arthurclemens.github.io/mithril-slider/#/carousel

TimMensch commented 7 years ago

I took inspiration from your change to add a "center" option to my fork (felt like an idiot for doing it the way I did), but I'm doing something you're still not: Automatically centering the item based on the given widths of the items.

I don't want to pull j2c in as a dependency, so I won't have easy access to an "offset" in code. And I may want that offset to change based on @media queries that change the size of the container or the images, or have varying sized images. So I really don't want a constant offset.

My latest version handles the auto-centering by just looking at the width of the item and the width of the container. Much cleaner. It needs to do one extra update to grab the size of the container after the child is created, but that was easily handled.

I can look at potentially merging my changes and sending a pull request if you like. But still too busy to do that right now.

Let me know if you'd be amenable to such a change. I certainly don't want to do the merge work if you don't like the change.

ArthurClemens commented 7 years ago

I am certainly interested. My centering solution is rather ad hoc, so I am sure it can be improved.

An alternative approach is to make the offset a view option that can be updated on redraws.

About j2c: if you use mithril-slider you are already pulling in j2c. It is really small, 2.4 K.

TimMensch commented 7 years ago

It's not pulling in j2c. I just checked. Only HammerJS, which I'm also considering prying out -- or using to replace touch event handling entirely. The engineer in me hates there to be two solutions to the same problem fighting with each other in the same project. The app is a game and currently uses Phaser, which has its own high speed event handling -- but in that "someday" future when I have more time I'm considering simplifying down to PIXI.js, in which case I might want Hammer... Well, someday.

The examples are pulling in j2c, but I'm not using it.

At some unspecified point in the future I'll try to come back to this and send in a pull request. Since we've diverged a bit, it may be a pain, but I'm sure I can figure it out. Maybe you'll have updated to Mithril 1.0 and I'll want to do the same, and I can just add the math into your version.

ArthurClemens commented 7 years ago

It's not pulling in j2c

It does in version 0.4.0

TimMensch commented 7 years ago

That's unfortunate. I guess that's another debate, but I really hate pulling in extra client code that I plan to never use. I don't care nearly so much about build systems, but every line of code sent to the client is extra overhead. I would far rather see raw CSS in the build than an entire library, especially since it's only like 10 lines of CSS that doesn't even take advantage of any runtime modification of CSS parameters.

I guess I'll keep my version.

ArthurClemens commented 7 years ago

The small benefit of j2c in this library is a fair point that will reconsider.

On 7 Feb 2017, at 23:12, Tim Mensch notifications@github.com wrote:

That's unfortunate. I guess that's another debate, but I really hate pulling in extra client code that I plan to never use. I don't care nearly so much about build systems, but every line of code sent to the client is extra overhead. I would far rather see raw CSS in the build than an entire library, especially since it's only like 10 lines of CSS that doesn't even take advantage of any runtime modification of CSS parameters.

I guess I'll keep my version.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or mute the thread.

ArthurClemens commented 7 years ago

And I have updated the code accordingly, in 0.4.1.