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

Extensibility #71

Closed
qurben opened this issue Jan 28, 2019 · 5 comments
Closed

Extensibility #71

qurben opened this issue Jan 28, 2019 · 5 comments

Comments

@qurben
Copy link

qurben commented Jan 28, 2019

I'm loving the ts rewrite so far!

I was wondering how to properly extend JGallery. Currently I am porting an implementation using JGallery 1 to JGallery 2, in the original implementation we used a lot of jquery trickery to modify the jgallery instance. With Typescript it seams more reasonable to extend the JGallery class or create decorators like withSlideShow.

When trying to extend the JGallery/Gallery class I bump into the following:

in gallery/index.ts:151 the create factory method is defined, but it strictly instantiates a Gallery class.

return new (compose(decorators, Gallery))(albums, params);

So when extending a class I cannot use the create method, but this way I lose the GalleryDecorators defined in the factory function.

There is also no way to pass additional GalleryDecorators when creating a JGallery instance. I'd also rather use the built jgallery.js instead of the typescript files.

What I would like is either:

  • When calling create on a subclass of Gallery that class is instantiated, which can be done by changing te line mentioned previously.
return new (compose(decorators, this))(albums, params);
  • Or for the ability to define additional decorators when calling the factory function.

Something like

static create(albums: Array<Album>, params: Params = {}, externalDecorators: Array<GalleryDecorator> = []): Gallery {
        const decorators: GalleryDecorator[] = [];

        params = { ...defaults, ...params };
        if (params.browserHistory) decorators.push(withBrowserHistory);
        if (params.slideShow) decorators.push(withSlideShow);
        if (params.thumbnails) decorators.push(withThumbnails);
        if (params.canChangePreviewSize) decorators.push(withPreviewSizeChanger);
        if (albums.length > 1) decorators.push(withAlbumsMenu);

        decorators.append(externalDecorators);

        return new (compose(decorators, Gallery))(albums, params);
}
@jakubkowalczyk-pl
Copy link
Owner

jakubkowalczyk-pl commented Feb 6, 2019

What about solution like below? Is it enough?

const createDecoratedGallery  = (albums: Array<Album>, params: Params = {})  => {
        const gallery = Gallery.create(albums, params);

        gallery.getElement().appendChild(
            Gallery.createElement('<div style="position: absolute; top: 0; left: 0">decorated gallery</div>')
        );

        return gallery;
}

@qurben
Copy link
Author

qurben commented Feb 11, 2019

I really want to be able to hook into the gallery, similar to how it happens in with-thumbnails.ts. Our jgallery 1 version has a way to tag users in the gallery. This requires us to detect if the next image is shown, what the current preview size is, etc.

Trying to read all this from the dom is probably very brittle.

@jakubkowalczyk-pl
Copy link
Owner

I'll add this with next release.

@jakubkowalczyk-pl
Copy link
Owner

I commited this change. Check if is enough.

@qurben
Copy link
Author

qurben commented Feb 20, 2020

Finally got around to implementing a gallery on top of this. It works very well, with the decorators I can keep the changes concise and understandable.

The implementation is in https://github.com/csrdelft/csrdelft.nl/tree/master/resources/assets/js/fotoalbum if you're interested.

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

No branches or pull requests

2 participants