Posted by merlinofchaos on September 6, 2012 at 11:38pm
19 followers
| Project: | Panelizer |
| Version: | 7.x-3.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
Issue Summary
The IPE allows us to add buttons to the toolbar and this actually works pretty well, even though the backend code is less than pretty.
It would be pretty awesome if we could take advantage of this to give more information and control to the user; in particular, if they have access to edit the default panelizer for an entity that's using a default, we should let them.
Patch forthcoming.
Comments
#1
Here's a patch that does just that.
This requires a small commit I made to Panels -- without this commit, the only thing that won't work is that buttons in the toolbar won't properly update if you save a panel that was previously using a default but is now using custom. Otherwise everything else should work anyway. That commit is here: http://drupalcode.org/project/panels.git/commit/7a8bd4e796c15ab8360197d1...
#2
#3
My biggest note here is that I'm uncomfortable with the wording of "Customize this page's default" but I can't think of anything that is succinct that makes it clear what's going on here.
To elaborate:
If you use the "Customize this page" button, you will customize *this page*.
The other button will customize the default, meaning all entities using that default will be updated. That button only appears if you're currently using the default (since there's no way to tell how different you are from the default).
#5
On the top, I think this is a great addition to Panelizer and solves a problem that many site administers have in understanding what happens when they use the IPE. I chatted with a few UI folks around this and wanted to propose that we switch around a bit how the button placement works:
Change One: Remove Top Level Buttons
The concern here is that these buttons might not be totally appropriate for the main IPE bar. Here are some reasons:
Change Two: Move Options Under Customize
We can move these options (when appropriate) under the save bar so that they only appear when they are able to be used. Some reasons:
#6
I'd have to think about it, but I'm fairly sure that moving to the save/cancel might actually be impossible, because internally a different object is being edited. I have some code to cause it to switch out which object this is before you start the edit process. I'm not sure that switching the object out after you've already started the edit process would be possible.
Obviously I can't put the word "Panopoly" into any of the buttons in Panelizer because Panopoly is layered on top of Panelizer, not the other way around.
#7
Re Feasibility - I am happy to help dig into the code to see if it might be possible to do this on the save/cancel side if you point me in the right direction. I think that having this done at this step will clean up the UI a lot.
Re "Panopoly Page" - This is actually just the name of the content type. I think it would be nice to include which template/default is being saved.
#8
Right now, Panelizer has a page callback for the extra button that mimics the normal Customize page, but swaps out the cache and sets some variables within the cache to tell it to override.
This would have to change to a form alter on the IPE form and do its update on the submit hook. The actual save happens in panelizer_panels_cache_save() which is a Panels callback to save a display cache. You can see that it has some annoying logic to determine if it's saving a default panel or a panel attached to an entity.
Since it's the same display it might be possible, at this point I'm just not entirely certain -- but it might theoretically be possible that the switches set on $cache could continue to work that late, because the display is the same. My worry would be accidentally saving properties from the local panelizer object to the default panelizer object, which *should* all be the same but could have slight differences.
#9
This is a big UX improvement; I'm a friend of anything that saves me from explaining to site builders that default Panelizer displays are created in Config > Content Editing > Panelizer.
#11
If #5 is possible i think that would be a nice improvement.
The only thing that i think might be worth considering is appropriately permissioning these two new buttons so we can control their exposure. I think the panels/ipe experience has done a really good job providing the barebones and simplest set of config options that are most relevant to everyday end users and light site builders. Furthermore, the IPE experience is a significant improvement over the base Drupal paradigm so it would be a shame to fall prey to the same problem that has plagued Drupal aka too many "Intimidating" buttons that scare many users away.
#12
Okay, here's the new approach.
First, this requires a patch to Panels as well as Panelizer. (Panels IPE only allows the original save button to save).
Second, this doesn't have icons for the new buttons. The CSS was a little confusing, so I figured I'd put that off.
Third, this doesn't help with the 'Change layout' meaning there's a WTF because you can customize the default if you have an uncustomized entity, but if you change the layout that must be customized right now. Will have to work out how to fix THAT.
#13
I spent about an hour playing around with this and I think things are looking pretty good. There are a couple issues I was able to find though.
#14
Actually marking this back to needs review -- as it would be great to get someone else to verify the permissions issue.
#15
The permissions part isn't in this patch; that got moved to its own issue. #1772844: Allow More Granular Permissions in the IPE So nothing in this patch should change that.
I worry about making the text too long; really long button text is intimidating.
#16
I also did some review and think this is a great improvement and really solid addition to the Panelizer/Panels IPE magic. Just for completeness, I updated the Panels patch to add in the appropriate icons and marking RTBC.
In terms of the the language, I think the shorter pattern makes sense. The concept of "custom" and "default" make sense to a lot of users and this text can be changed on the theme layer if there is such a need.
In terms of the permissions, I assume the permissions patch will only allow the "Save as default" option to folks with that permission.
#17
I have been playing around with this more and was wondering if it might make sense to implement a JS "confirmation" for reverting to default or saving as default. Those are pretty powerful operations and might deserve a slightly higher level of confirmation. It is pretty easy to accidentally hit "Save as default" instead of "Save as Custom".
#18
I completely agree with the potential for needing a confirmation. While I was fiddling with it I was worried about people who blindly hit the wrong button.
#19
I don't think we can use IDs generated in Panelizer in Panels' CSS. That doesn't really help if something else, elsewhere, also adds new buttons.
#20
Re-rolled the panelizer patch against the latest -dev and added confirmation dialogs for the 'save as default' and 'revert to default' buttons. (Wording could perhaps use some work, I'm not sure how general it needs to be.) Thanks for the patch, this is very useful functionality.
#21
acrollet: Did your confirm javascript not get into the patch? (new files have to be added via git add and then use git diff --staged)
#22
it's in there as an onclick attribute of each form element - apologies if this isn't the best way to do it, I was just looking for a quick way to add the functionality and keep this issue moving :)
#23
Oooh. I try to avoid onclick and inline javascript, pretty much ever.
#24
duly noted! Happy to re-roll, can you give a hint as to your preferred method? (even just to say "figure out how it's done for the cancel button and do it that way" is enough)
#25
@acrollet: what you want is proper event binding added via jQuery. See the javascript behaviors documentation at http://drupal.org/node/756722#behaviors
#26
Getting the confirm to work right using standard procedures was surprisingly difficult. Given the time it took to accomplish this, I probably shoud've just kept with the onclick, but it seems to be working now.
I have cleaned this up a little, committed and pushed it. It requires changes also pushed to Panels in order for the icons and the confirm dialog to work correctly.
#27
Sorry if I should be creating a new issue for this.
Editing the default panelizer state from within IPE is not working for me. I am using the following versions of modules:
panelizer 7.x-3.0+0-dev
panels 7.x-3.3+33-dev
ctools 7.x-1.2+31-dev
If I override a particular node I can save as custom and revert to the default through IPE just fine but when I try to save as default it just saves it the same as it was prior to making any changes. Thanks.
#28
I am confirming the issue for #27. I am using the same set of modules:
When I attempt to save the panel as the default I get the confirmation screen several times. If I click "OK" the panel does is not updated and the default panel is also not updated.
#29
#27, #28: Are you using entitycache module?
#30
No, I am not using entitycache. I was actually able to reproduce it on a clean install.
#31
Ok, there were 2 separate bugs making that fail. I've attached patches for the purposes of verification, but both are committed to -dev. One is in Panels, one is in Panelizer.
#32
This appears to fix the issue on a clean install using the default view mode.
I am still having some issues on a different site with a different view mode but I will do more testing tomorrow to see if it is just me. Thanks!
#33
Cached javascript on the existing site could be an issue. Clear all caches?
#34
Ok, so it seems to work great on a clean install. The other site I am testing is a new site built with a custom install profile and my content types and panelizer defaults are all packaged up as features with demo content. When I try to save as default through ipe it appears to work at first but if I leave the node and come back it reverts to the defaults stored in the feature.
If I go into /admin/config/content/panelizer and save the panelizer settings for that particular content type without making any changes it will then work through the ipe. Any ideas?
#35
Hm. I'll have to duplicate that situation and see what's going on. Maybe something isn't succeeding in the save, though I can't imagine why. At least there's a workaround for the problem for the moment, even if it's a crappy one.