A patch to intergrate Flickity with React

[oxpa]

Hello.
First of all, I have to admit, I'm not a js programmer. I have no knowledge of npm or whatever. So this is more a feature request than a pull request =)
Still, this pull request consists of two changes.
The first one introduced 'noDomMod' option for Flickity. This option prevents DOM modifications inside Flickity.
Why would anyone need this? When I used Flickity with React I realized, Flickity changes DOM and React fails (because React checks that DOM matches React's internal shadow DOM). The change Flickity made is trivial: only two div elements are added and cells are moved into them.
So the noDomMod patch is about basic react compatibility. In my case, I used react to filter out only matching tabs. But here is a simplier example, which just adds and removes cells:
https://codepen.io/oxpa/pen/VQmNdN (sorry for a ton of JS, but this is how I'm using react...)

But there are several problems with this rendering. As you might notice, in the demo I'm using .activate() and deactivate() while I would prefer .reloadCells(). First of all, reload doesn't update dots. And I didn't work around this issue. Secondly, reload mess with cells if I don't reset before and after wrap arrays. And this is the second commit: normalize wrapping behaviour on external DOM management.

So, basically, could anyone please review this and propose a reloadCells better patch?
(I read the contributing guideline, and lets agree, this is a mit licensed pull request)

[desandro]

Thank you for this contribution.

As a non-React user, this is foreign for me. I'm not sure if I want Flickity to go down this path. Flickity was designed with a browser environment in mind, at nearly every level. Removing DOM from Flickity and its dependencies would be a complicated endeavor. Even then, I'm not sure how successful Flickity would work in a server-side environment. It simply was not built for this use case.

I appreciate your effort here, but I do not feel comfortable merging in this code. I'll keep this PR open for a bit so others can provide feedback.

[PaoloFalomo]

In my opinion I find it a bit useless.
This seems to be a fusion of two too much different worlds.

I think that this feature would be achived only rewriting (a big?) part of the flickity's code.

Btw i liked the unusual purpose.

Just my opinion, do not kill me please 😂.

[oxpa]

@desandro afaik, Flickity can't work in a server environment at least because it uses client side features moving slides back and forth. But this is not the case ;)
I also don't propose removing DOM operations totally. But to have a way to tell Flickity that environment is set up as Flickity would need: a viewport, with slider, with cells inside. Just like Flickity assumes there are cells inside anchor element.

By the way, initially, I used reloadCells, and it almost worked ;) But iit doesn't really reload cells. It does a part of this realoading, while deactivate-activate does this task better...

@PaoloFalomo I'm not the first one. There is at least one gist in google for this =)
https://github.com/theolampert/react-flickity-component of @theolampert
https://gist.github.com/JoshBarr/f302ebf7f43ea596ad6a of @JoshBarr

Both of those assumes cell configuration is static. While if you have to filter out some cells and reload them, Flickity behaves not that well as you might want it ...

So, as a resume: this is not intended to be in a server side render. Nor this is a replacement for DOM manipulation in Flickity. This is a small feature to delegate some work to an outside framework.

[PaoloFalomo]

@oxpa
is this working with multiple flickity sliders too?
querySelector() references only to the first element found with that selection but if there're more '.flickity-viewport' (eg. with more than 1 slider) it will take just the first found discarding others.

I'm just mind-debugging and i think that could ✅ work if you run the querySelector() in an inner element instead of looking throw all the document.

[oxpa]

Hm... You are right: I didn't keep in mind, there might be several instances. I'll have this changed in awhile

[oxpa]

I have updated the patch and example to work with multiple Flickity instances. (Still they share react state so will have the same amount of cells, sorry =)

And I'm still not sure about the 'reloadCells' hunk.

[claydiffrient]

Oh wow @oxpa thanks so much for this! I've been fighting all day with how to get Flickity to work inside of a React app. This did the trick!

@desandro Please consider accepting this or something quite similar. React handles all DOM manipulation itself via a virtual DOM implementation. When a plugin/library/etc. modifies the DOM outside of that, React doesn't really know what to do and many times will fail. By providing the noDomMod option React can handle the .flickity-viewport and .flickity-slider nodes while allowing Flickity handle everything else.

[robinpyon]

Just echoing the above, this patch has been helpful for me integrating Flickity in a React app with SSR

[smartmike]

This is similar to #488 – I handle the addition/removing cells in React though, if the props that I'm rendering my children with, I call reloadCells and updatePageDots if needed

[wolthers]

+1

This patch is also required to get Flickity to work in an advanced VueJS setup. Same problems, same solution.

[wolthers]

To offer a perhaps slightly different explanation of the problem that this PR is trying to solve:

Libraries like React and Vue which rely on virtual DOM implementations (an object tree structure that mirrors the actual DOM), get confused when you modify DOM structure manually because the actual DOM no longer mirrors the structure of the virtual one.

Flickity transforms your markup on instantiation by wrapping your cells in .flickity-viewport and .flickity-slider. This PR allows the user to specify that this markup structure is already in place before instantiation, so Flickity shouldn't worry about doing it. Because Flickity no longer transforms your markup with this option enabled, the virtual DOM and the actual DOM never gets out of sync and React and Vue keep working.

More and more developers are moving towards libraries that take advantage of virtual DOM, so I think it is definitely a valid use case for Flickity.

[desandro]

@wolthers Thanks for providing more insight on this PR. I'm still evaluating the best implementation.

[hinok]

@desandro Could you share with us your plans in terms of support for integration with react? I really consider buying commercial license but React support is a must-have in my case. Happy new year! 🎉 🎈 🍾

[kapouer]

This PR is also a must when trying to integrate flickity with prosemirror's nodeView. It allows one to edit this carousel in a wysiwyg editor (granted, you need a certain amount of work to achieve this).

[nullablebool]

👍 +1 For this PR.

The changes are pretty innocuous. The offending elements that are added by Flickity, namely; flickity-viewport and flickity-slider, are not consumed in a way that a wrong declaration can break Flickity. Unless there are some future plans which render this assumption false then I propose an option of sort is made into the next release.

I would suggest a more meaningful name, noDomMod works, but something more explicit like preventDomTreeManipulation, or even allowing both viewport and slider to be passed in as options.

Another suggestion, specifically for this patch. Is that I don't agree with the suppression of both this.element.removeChild( this.viewport ); and moveElements( this.slider.children, this.element );. As far as I am concerned, although we created this element - it is coupled with Flickity and on deactive it ought to be removed.

Nitpicking now 😄 I would consider using throw over console.error. It looks like Flickity doesn't throw any errors but I'm not familiar enough to know if there are currently any configurations that could break instantiation. As these configurations are paramount to Flickity's instantiation then throwing may be preferable: throw ('Flickity: preventDomTreeManipulation set to true but could not find ".flickity-viewport"');

[smartmike]

This is probably relevant here too: #488 (comment)

[yaodingyd]

Self-promotion here: please check out react-flickity-component for using with react.

[jonnitto]

Also for working in CMS, would this be a very nice and essential feature. When do you plan to merge this?