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

fix: Recollect slides during update to support dynamic additions, removals and reordering #457

Merged
merged 1 commit into from
Nov 22, 2021

Conversation

driskell
Copy link
Contributor

@driskell driskell commented Feb 14, 2020

Currently, if you add new slides or remove slides, and then call update(), carousel will completely break with strange behaviour. This is because the Components.Html.slides property still contains removed slides, and does not contain new slides. This means clones can get added into the wrong place (if the initial slide or last slide was moved for example) and also means clones are incorrect. Furthermore, new slides don't receive a width style so display completely the wrong size.

This PR implements updating of slides during update(), and thus the Sizes and Clones components now work correctly after new slides are added, old slides are removed or existing slides are moved.

I have also implemented a functional test for this, which fails when the Html component changes are reverted. It checks that the width style is properly applied to a new dynamically added slide.

This PR will allow Glide to be safely used in a React environment. One just needs to call update() during componentDidUpdate either every time or when slides are known to have been changed. Using hooks this is even easier by calling update() in a useEffect(), with dependencies on any props that affect slides.

Related issues:

@driskell
Copy link
Contributor Author

As a temporary workaround the following code can be used to patch the running version of Glide. Due to the fact @glidejs/glide import is a bundle, and the imported Html is not, it does duplicate some of the code, but not a huge amount.

import Glide from '@glidejs/glide';
import Html from '@glidejs/glide/src/components/html';

function HtmlFix(Glide, Components, Events) {
    const HtmlFix = Html(Glide, Components, Events);
    Events.on('update', () => {
        HtmlFix.mount();
    });
    return HtmlFix;
}

let glide = new Glide(element, options);
glide.mount({Html: HtmlFix})

@driskell
Copy link
Contributor Author

driskell commented Apr 6, 2020

Hi @jedrzejchalubek Hope you are OK - strange times.

Is there anything needed here to get this into a release? It'll be really good to do so as it makes React integrations much much easier as slides can be properly updated dynamically.

Thanks!

@whazlewo
Copy link

whazlewo commented Aug 3, 2020

I'm looking forward to this this being added! If I'm reading this right, it will make it so I can add/remove/reorder list items and then call glide.update() and have the magic happen.

@driskell
Copy link
Contributor Author

driskell commented Sep 4, 2020

Hi @jedrzejchalubek

If you’ve got a small moment I’d be happy to hear feedback. Test is included too so hopefully this is a suitable PR ready to merge. It would greatly enhance Glide and its compatibility with dynamic implementations.

Thanks!

@driskell
Copy link
Contributor Author

Hi @jedrzejchalubek

Is there any way I can help?

@FlorianWerndl
Copy link

FlorianWerndl commented Nov 3, 2020

Thanks for the workaround @driskell

If anybody is using this in a vuejs setup and needs IE11 support, you eventually need to add @glidejs/glide to the transpileDependencies in your vue.config.js.

@egogoger
Copy link

egogoger commented May 8, 2021

I tried the patch suggested by @driskell but for some reason it didn't work on every glide.update() so instead I used this workaround. Basically you update glide by making React re-render your wrapper.

const GlideWrapper = (props:IProps):JSX.Element => (
    <div key={props.children.length}>
        <Glide {...props}>
            {props.children}
        </Glide>
    </div>
);

@driskell
Copy link
Contributor Author

Closing as it looks like this won't be merged and we're migrating away from glidejs in order to meet accessibility requirements.

@driskell driskell closed this Aug 11, 2021
@jedrzejchalubek jedrzejchalubek changed the title Recollect slides during update to support dynamic additions, removals and reordering fix: Recollect slides during update to support dynamic additions, removals and reordering Nov 22, 2021
@jedrzejchalubek jedrzejchalubek merged commit ded7e74 into glidejs:master Nov 22, 2021
@jedrzejchalubek
Copy link
Member

A little bit late, but re-opening and merging it to the master. it is a nice and clean solution and great explanation of problem, thanks 👌

@mdesignfa
Copy link

(Resolved ✅)
Hi there...

I found an easy-to-use solution for this issue!
the key to add or remove slides into your glider js is => Before adding or removing slides you must call destroy() Method then add your new-dynamic slides then you must initialize your Glider again! (worked 100% for me)

See the Screen-Shot below
done

if your looking for more practical Example please see the below Rep:
https://github.com/mdesignfa/add-or-remove-slides-form-Glider.js

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

Successfully merging this pull request may close these issues.

6 participants