Closed (fixed)
Project:
Views Slideshow
Version:
6.x-2.x-dev
Component:
Code
Priority:
Critical
Category:
Task
Assigned:
Reporter:
Created:
30 Mar 2010 at 02:57 UTC
Updated:
27 Apr 2010 at 06:30 UTC
Jump to comment: Most recent file
Since every module has to declare a fieldset for the settings form it's easier to just declare it in the main module. This will change the api and break current contrib modules. But this should clean things up a bit. Patch coming soon.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | 756908-fieldset-4.patch | 36.05 KB | redndahead |
| #6 | 756908-fieldset-3.patch | 36.08 KB | redndahead |
| #2 | 756908-fieldset-2.patch | 32.4 KB | redndahead |
| #1 | 756908-fieldset-1.patch | 3.61 KB | redndahead |
Comments
Comment #1
redndahead commentedHere is the first stab at the patch. Not really happy about it because it does some funky things to get it to be compatible with older betas.
If we were to re-implement this I would require the form key to be the module name. So instead of $form['singleframe']['blah'] it would have to be $form['views_slideshow_singleframe']['blah']. The problem with changing this is that anyone that is currently using 2.x will lose the settings that they have in views_slideshow currently. I think that will make people grumpy. I don't know if it's possible to run an update query against the views variables, but if someone has an idea that would be great.
Comment #2
redndahead commentedHere is the patch for the other option. This one needs an update function for the variable name changes.
Comment #3
redndahead commentedOK we have a somewhat serious problem that needs a solution here. Currently the way this module is set up if you want to add a contrib module it must be named views_slideshow_*** We shouldn't limit it to this naming convention. Patch #2 removes this limitation, but at the same time it requires a change to all the variable names and an upgrade path doesn't look likely. So we have 2 choices that I see.
1) Make the changes with no upgrade path and tell everyone that they need to reset their settings after upgrading.
2) Make the changes and check for the time being adjust for the old values if they exist. This could be complicated especially with exported views.
Either way I don't think we're going to get much love from people. I'd love to hear any suggestions.
Comment #4
redndahead commentedI feel like I'm talking to myself, but I have decided to go with #2.
I haven't been able to find a way to migrate old settings to new ones so when I make this change I'll post that the settings will have to be recreated and saved. This is going to cause problems all around, but this removes a severe limitation of this module.
If anyone has any ideas on how to migrate the settings I'd be happy to hear them.
Comment #5
redndahead commentedLearned way to migrate old settings. Posting notes here so I can come back and look at them.
Look at lines 83-117 and see how we can replace the variable on init.
http://drupalcode.org/viewvc/drupal/contributions/modules/views/plugins/...
Will need some good testing, but this should make for a release of beta-3.
Comment #6
redndahead commentedHere is the final patch that's to be committed. This patch will make it so the user won't have to update anything.
Comment #7
redndahead commentedThis has some indentation cleanups.
Comment #8
redndahead commentedThis has been committed.