Closed (fixed)
Project:
Galleria
Version:
7.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
18 Jul 2011 at 09:30 UTC
Updated:
4 Jan 2014 at 01:11 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
gmclelland commentedThis would be awesome. I've been looking into this a bit and it looks like the http://drupal.org/project/flexslider module already has this functionality. In fact the code and concepts are very similar. For instance, flexslider has themes and option sets. The db schema looks very similar.
I think a lot of code can be copy and pasted with minor adjustments.
Comment #2
kroimon commentedYeah, would be great if someone could do that.
I unfortunately just don't have the time to do it myself at the moment :-/
Comment #3
gmclelland commentedHere is my first go at it. I'm attaching the code as a zip because I haven't figured out yet how to pull in the latest commit without messing up the work I have done in git.
I will see if I can get a patch for you.
Changes made:
dependency on ctools
works with ctools bulk exporter
works with features module
added a new permission (Administer the Galleria module)
added a new image style (galleria_full_screen)
uses ctools exportables to provide a default optionset
Hope that helps,
-Glenn
Comment #4
gmclelland commentedHere is the patch against the latest dev release.
Comment #5
kroimon commentedThanks for your work, gmclelland! I just tried the patch and it seems to work great!
However, I have to adjust it to the latest changes before I can commit it, and I have a question and a request:
Is the new 'administer galleria' permission necessary? Or can we separate this into another commit or even leave it as it currently is?
And my request: This 'A form based import/export mechanism will be added in the future.'... Are you able to implement that, too? :-)
Comment #6
gmclelland commentedIs the new 'administer galleria' permission necessary? Or can we separate this into another commit or even leave it as it currently is?
-I was just going by standard Drupal practice. If you want to separate it out into another commit, that's fine with me. This module is very similar to the flexslider module which has it's own permission.
And my request: This 'A form based import/export mechanism will be added in the future.'... Are you able to implement that, too? :-)
-I basically took everything from the flexslider module and adapted it to work with the galleria module. I'm not sure how to add the form based import/export. Feel free to remove/change that message.
The patch I posted seems to work for me, but I would like to see a better implementation of it. Here are my concerns with the patch I posted:
1. I think I changed the database table name from galleria to galleria_optionset - this is how flexslider does it, but we might need to make some changes so that it doesn't break existing installations or make changes so that it just uses galleria as the db table name.
2. I'm not sure how it will affect existing installs since I added a default optionset using ctools exported to code. We might need to write an update function that deletes the default row in the galleria database table. If people are using and have customized the default optionset, their changes would be reset back to default.
3. The patch I posted uses ctools exportables, but doesn't use ctools export_ui. I think a lot of code could be reduced by using ctools export_ui. ctools export_ui handles the listing, adding, cloning, reverting, deleting, importing/exporting, of optionsets with a search filter on the optionset overview/list page. Basically just like the views module does. It does all of this by reading the db schema. I started to work on this but got stuck on creating the add/edit optionset form. I had the listing page done with filtering, sorting, reverting, cloning, deleting, import and exporting, but without the add/edit form working it doesn't do any good.
If your interested in the export_ui stuff, I can show you what I have.
Comment #7
kroimon commentedNo, your patch doesn't change the table name. It is still galleria_optionset as it was before, so the migration is straightforward.
Also, the update doesn't delete the default optionset, we just don't need to add it to the database any more on installation as it is stored in code. Existing modifications to the default optionset get detected automatically and override the hardcoded values as expected.
All in all, I could just commit your patch as-is and it's still 100% backwards-compatible, except for the added dependency on ctools, which gets compensated with the new import/export features and the ability to revert the default optionset to the installation defaults.
The export_ui features sound cool and definitely worth a look. Maybe you could checkout the ctools-branch in the project repository and adapt your export_ui stuff to it :-)
Comment #8
gmclelland commentedNo, your patch doesn't change the table name. It is still galleria_optionset as it was before, so the migration is straightforward.
Also, the update doesn't delete the default optionset, we just don't need to add it to the database any more on installation as it is stored in code. Existing modifications to the default optionset get detected automatically and override the hardcoded values as expected.
--I couldn't remember exactly, I'm glad this works.
All in all, I could just commit your patch as-is and it's still 100% backwards-compatible, except for the added dependency on ctools, which gets compensated with the new import/export features and the ability to revert the default optionset to the installation defaults.
--Sounds good to me. We can always replace it with the export_ui stuff later without any problems.
The export_ui features sound cool and definitely worth a look. Maybe you could checkout the ctools-branch in the project repository and adapt your export_ui stuff to it :-)
--Will try.
Comment #9
gmclelland commentedFYI.. Here is the interface that export_ui provides.
See attached image
Comment #10
reubenavery commentedHi all,
@gmclelland -- thank you for this.
Please find attached your patch modified to work against the latest 7.x-1.x-beta3 and dev releases.
Comment #11
kroimon commentedI just pushed these chages live to the -dev branch, so they are included in the next -dev build.
When updating, please don't forget to run the update.php and make sure you have a recent version of the ctools module installed.
Comment #12
kroimon commentedI close this issue for now.
@gmclelland: If you want to provide a patch with your export UI support, please open a new issue :-)