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

Galleria is always present and cannot be disabled in plonetruegallery #53

Open
fredvd opened this issue Apr 13, 2016 · 1 comment
Open

Comments

@fredvd
Copy link
Member

fredvd commented Apr 13, 2016

We prefer to use a different slideshow library in our installations and not allow our users to choose a different library we don't support.

However collective.ptg.galleria is a hard dependency in the setup.py of collective.plonetruegallyer and the default value of display_type in IGallerySettings is galleria, thus Galleria is always there :-(

What would be the best route to support enabling this? We could make the dependency on galleria optional in the setup.py using an extras_require, but that would need an explicit warning on a new release that people should change the package include to collective.plonetruegallery[galleria].

Also the default value in IGallerySettings would have to be removed. The value list is already created using a vocabulary, so the default value would be the first item in the list of actually installed collective.ptg.* packages.

My idea is to add an extra option in the generic plonetruegallery control panel where you can select which of the found DisplayTypes should be activated and choosable. With an ordered in/out widget this could also solve the default item: move the default item to the top of the enabled list. (A similar construct is used in for example collective.cover to select from the found available tiles in the application should be available to the users. Sorry for the Dutch screen shot)

tilechooser

@mauritsvanrees
Copy link
Member

We could keep galleria in the install_requires for now, at least until a next major release, or until maintainers tire of it.

And already introduce a [galleria] extra that has the extra dependency, so people who know they want to keep galleria can already specify this extra in their buildouts, without fear of suddenly losing galleria in a new version.

I had a go at seeing what it would be like to implement this in the way you propose. It should be doable. The tricky thing is that the IGallerySettings interface is used for both the control panel and the gallery on the context. The enabled display types should only be settable in the control panel.

And there is a __getattr__ override in the settings, which is always nice... It gets the value from the context and when it is not there it gets the value from the control panel and when the default has never been changed here, it takes the default from the interface. And with what we would add, this default would get its value from a new enabled display types vocabulary, which itself will have the current display types as default. I think... I ended up in a recursion error already, with a defaultFactory trying to get a value from a vocabulary, which has a default factory, etcetera. So it is fallback on fallback on fallback, which makes it a bit hard to keep it all in your head...

Having two fields already makes it easier to think about: so one for the enabled types, and one for the default type, instead of putting those two in a single ordered in/out widget.

So: implementation could be a challenge. But before we put more work into this: would this feature be acceptable?

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