Comments

damienmckenna’s picture

Status: Active » Needs review
StatusFileSize
new1007 bytes

Step 1: Use a variable to select the default loaded on the node-edit page.

damienmckenna’s picture

StatusFileSize
new2.66 KB

This patch provides both the logic to select the default and assign it, adds a new 'default panel' item to the settings instead of creating a new variable, and defaults to the normal "{entity_type}:{bundle}:default" panel to retain complete backwards compatibility.

damienmckenna’s picture

I'm particularly interested to know if there are specific use cases that aren't covered for or possible glitches.

damienmckenna’s picture

Might it be worth improving the UX to indicate on the overview page which display is the default?

damienmckenna’s picture

Issue tags: +Needs usability review

Tagging that this could use a usability review.

damienmckenna’s picture

Title: Allow default Panelizer to be controlled » Allow default Panelizer to be selected
damienmckenna’s picture

Issue summary: View changes
damienmckenna’s picture

StatusFileSize
new34.56 KB

FYI this is what it looks like:

Screenshot of the new selector

damienmckenna’s picture

StatusFileSize
new5.31 KB

This is an updated patch that goes back to using variables to control which display is used, allowing the selection to be exported.

damienmckenna’s picture

StatusFileSize
new5.25 KB

Rerolled.

damienmckenna’s picture

StatusFileSize
new5.23 KB

Missed a few variables when I changed he separator to a colon instead of a single underline.

damienmckenna’s picture

Issue tags: +Check with merlinofchaos
damienmckenna’s picture

damienmckenna’s picture

Status: Needs review » Needs work

This needs some work:

  • Allow no default to be selected.
  • Changing handling of the "Provide default panel" & rename it ("Provide initial display") so it only generates a display, it doesn't override this new setting.
  • Indicate the storage status of each one (just 'cause).
  • Update help message to make it clear how this plays with the others.
damienmckenna’s picture

Status: Needs work » Needs review
StatusFileSize
new8.62 KB

This has been updated to include the changes noted in #15. I've realized that this changes things slightly, the creation of a 'default' display is now different to the selection of the default.

damienmckenna’s picture

StatusFileSize
new8.94 KB

This resolves a problem with the patch in #15 where selecting "Do not assign a default" made no difference on the entity form selector.

damienmckenna’s picture

With the patch in #16 this is what the content type options look like:

Content type settings.

The node form then has the following selector:

Node form.

damienmckenna’s picture

The gotcha right now is the upgrade process. I think this needs an update script to identify any entities / bundles that have the "default display" option enabled and assign it as the default via the new variable.

Any thoughts?

damienmckenna’s picture

Items to debate:

  1. Do we need an option to control whether each an item must be displayed? I think we do.
  2. Shouldn't the entity form always have the "-- No panel --" (option unless we require one be selected)? I think so.
  3. If the user selects the default display should the panelizer_entity record be saved to note this? If so, what if the default display is changed later on, should the old records also be updated later on or should the system lock those older records to the older display? My guess is that default values should not be saved and that older "default display" nodes should adapt the new default.
  4. Is an update script needed to assign the new "default" variable for any entity/bundle that has the previous "default display" option enabled? I think so.
dsnopek’s picture

This seems like a useful feature! I'm not sure we'll need it directly in Panopoly, but certainly it'll get used in Apps or distributions based on Panopoly. I know that Open Atrium 2 uses the multiple Panelizer default functionality extensively, so it'd probably be good to bring them into this discussion as well!

As to my opinion on the questions in #19:

1. I don't entirely understand this one - do you mean control whether each default is an option for being the default Panel?

2. I think so too.

3. Yes, I agree: if later you update the default Panel, then all the old entities which used the default should adapt to it.

4. Yes! This is probably the most important point, however, more so for direct users of Panelizer and not distributions. For a distribution like Open Atrium, they'll simultaneously update their Features when they update to a new Panelizer, which will set the variable - so there won't be any need for the update.

Anyway, I hope that helps! Unfortunately, I haven't had a chance to test or review the patch, just read the comments here and look at the screenshots.

damienmckenna’s picture

Status: Needs review » Needs work

I have an idea on how to change this so it is completely backwards compatible and wouldn't need an update script. I'll try to get to it this week.

damienmckenna’s picture

Ok, I've realized there's a problem. Each entity currently tracks which display is being used; this selection saved in the {panelizer_entity} table. The problem is that it saves the full name of the display, e.g. "node:article:default", rather than just "default", so if the default is changed via this new option, any entity that was using the default will not be updated to the new default. What we'll need to do instead is change the default handling to store a placeholder value, e.g. "default" and then have the display loading logic identify the appropriate default at loading time.

I've got the current patch tidied up, but I need to update it to handle the placeholder, and add an update script to fix existing records.

damienmckenna’s picture

StatusFileSize
new10.82 KB

Just so I don't accidentally loose it, WIP that needs to have the placeholder functionality added.

damienmckenna’s picture

Related: #1965148: Load the full Panelizer default object - this would stop saving records if the default is selected, and simplify the work on this issue.

damienmckenna’s picture

StatusFileSize
new15.26 KB

I hadn't uploaded this before, I thought I'd better before I accidentally loose the changes again. It still needs work =)

damienmckenna’s picture

Status: Needs work » Needs review
StatusFileSize
new13.54 KB

Now that the default value handling is being taken care of by another issue, this becomes much simpler.

damienmckenna’s picture

StatusFileSize
new13.93 KB

This version adds a small little UX improvement to indicate in each selector on add_bundle_settings_form and settings_form which one is the current default.

damienmckenna’s picture

It'd be nice to be able to indicate on the settings overview page which display is the current default, but I don't know if there's an easy way to tap into ctools_export_ui->list_page without just duplicating the whole function.

damienmckenna’s picture

Issue tags: -Needs usability review

Removing the "usability review" tag as the default display aspect is no longer being handled by it.

damienmckenna’s picture

StatusFileSize
new13.91 KB

This fixes a bug where I forgot to define an $options array, and removes a todo comment for something that now works.

damienmckenna’s picture

StatusFileSize
new16.59 KB

Needed a change in hook_entity_load() to use the new variable (must have missed it during a reroll). Also, add_bundle_setting_form_submit() was updated so that if EntityCache is installed it will clear the caches if the default is changed, that way all such entities will have their display reloaded.

damienmckenna’s picture

Status: Needs review » Needs work

This needs some work, I was having some problems with it this morning, so something must have been lost again during the reroll.

damienmckenna’s picture

Status: Needs work » Needs review
StatusFileSize
new16.28 KB

Rerolled, along with some fixes and testing.

damienmckenna’s picture

Issue tags: -Check with merlinofchaos
StatusFileSize
new16.31 KB

Rerolled.

damienmckenna’s picture

StatusFileSize
new19.5 KB

Some small UX improvements around the default selector.

damienmckenna’s picture

StatusFileSize
new19.23 KB

Contains a minor tweak to fix the problem that creating new entities did not select the correct default, and there's no reason to add "(selected)" to the previously selected item when editing an entity.

damienmckenna’s picture

StatusFileSize
new1.42 KB

Interdiff for patch #36.

damienmckenna’s picture

StatusFileSize
new19.34 KB

A minor wording tweak in add_bundle_setting_form().

damienmckenna’s picture

Status: Needs review » Fixed

Committed. There are several related issues slated for v3.2 (see #2155813: Plan for Panelizer 7.x-3.2 release) that will help simplify the UX around this.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.