Recognize disabling of view, & warn that image gallery admin settings have no effect
joachim - January 7, 2008 - 17:42
| Project: | Image |
| Version: | 5.x-2.x-dev |
| Component: | image_gallery |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
If Views is being used, the settings at admin/settings/image_gallery do nothing.
Patch to add a warning to this effect.
(Overriding the entire form seems too complicated, as the call to system_settings_form supplies buttons that then are out of place.)
| Attachment | Size |
|---|---|
| image_gallery_adminnote.patch | 845 bytes |
#1
can we just conditionally display the form? if views is enabled we hide the form and just display a note saying that all the config is done via views and provide a link?
#2
#3
The menu item callback would need to be conditional, as it's currently a 'get form'. Is that advisable -- does it create cache issues, for instance?
#4
it shouldn't cause problems. enabling/disabling views module would cause a rebuild of the menu cache.
#5
Updated patch.
Two different messages, depending on whether Views_UI is there too.
#6
i'm wondering if we should check that the image_gallery view exists in addition to checking that the view module exists... that why if people want the old code they can just disable the view. what do you think?
#7
I wonder what happens if people disable the view actually.
They might get no galleries at all...
I'll look into it & file a separate issue.
#8
Actually, I'm going to do this as one patch -- hope you don't mind, but my version is all tangled up with several things at once. I hope I've stripped out the menu tabs changes!
This patch:
- recognizes if the admin has disabled the view, and if so, uses the old built-in system (I've tested that overriding the view works properly; Views doesn't count this as disabled)
- if the view system is being used, shows a note on the galleries admin page as in the previous patch.
- since checking this is a bit of a mouthful, I've added a function image_gallery_is_using_views()
#9
Are you very certain of the result ? I think a caution is worth it.
#10
Sorry, could you explain what you mean?
What result? What image_gallery_is_using_views() returns?
#11
Marked http://drupal.org/node/214249 as duplicate.
#12
I tried to apply the patch in #8, and got
File to patch: image/contrib/image_gallery/image_gallery.module
patching file image/contrib/image_gallery/image_gallery.module
patch: **** malformed patch at line 21: @@ -73,6 +74,31 @@
Anybody else?
#13
OK, cleared the cache, and the patch applies, and works a treat.
#14
Ah yes, this is the patch which applies cleanly ...
... and the INSTALL.txt modification, too.
I wonder, would it be better to name the gallery views "views_gallery_default.inc", and the block views "views_blocks_default.inc"? That'd remove redundancy, for the gallery views.
#15
I don't understand your patch on INSTALL.txt.
There's no need to copy anything to /includes/, and contrib modules shouldn't mess with Drupal's core folders anyway.
The two .inc files are loaded up by the image gallery module itself. I'm following drewish's practise of having code that depends on other modules separated into inc files, as he's done in image.module.
Also, what do you mean about renaming? Do you mean change the name of the files, or change the system name of the views?
#16
Really? I've copied them over, just because. I'll delete that patch.
The renaming is moot, then, too.
#17
how does this look?
#18
Lovely, works a treat.
#19
Isn't that patch going to produce redundant Submit / Reset buttons, because you're within the form generation code?
That was why I put it into a separate function.
#20
joachim, no it shouldn't, the
return $form;breaks out before the buttons are added.#21
Seems fair enough -- let's commit before it rots :)
#22
done, thanks for the reminder.
#23
Automatically closed -- issue fixed for two weeks with no activity.