The latest Panopoly switches to the new "Single Preview" mode by default in the Add Widget dialog when customizing a page. The problem with this list is it doesn't give the user much information about what each pane is intended for. I suggest adding the Description of the pane to the output. Here is a sample screenshot (using the Open Atrium Radix theme, but you get the idea)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mpotter created an issue. See original summary.

mpotter’s picture

Status: Active » Needs review
FileSize
1.49 KB

Here is a patch for review

dsnopek’s picture

I think including the description would be great! The screenshot looks great to me, although, I'd like to get some more input from other folks. It might make sense to make this optional (enabled on new sites and disabled on upgrade sites) to not cause problems on upgrade.

Also, we'll need to get Behat tests for this and some manual testing under vanilla Panopoly and Bartik.

mpotter’s picture

Sure, I can see adding an option, although since it only affects the Single Preview I'm not sure how many existing sites it would affect since Single Preview has only been the default for a short time. I'd honestly enable it on new sites though since I find it a much better user experience than just the raw list. FYI we are using this patch in Atrium 2.50-rc2.

And yes, definitely needs tests.

dsnopek’s picture

Ah, yeah, I'm all for enabling this on new sites, I just don't want to do something unwanted when upgrading old sites. I type-o'd in my original comment -- I'll edit it in a moment.

cboyden’s picture

This is looking good. I took a look at view pane descriptions in the Add Content dialog in single preview mode, and those are being displayed as expected.

I'm not sure there's going to be a problem on upgrade, it doesn't seem like a disruptive change and it will only affect people who are in single preview mode already. But if you want to make this a setting option, it could be enabled only for new installations.

dsnopek’s picture

Yeah, now that we're moving into "maintenance mode" I'm really paranoid about making unexpected changes for existing sites. I'd like for site owners to just be able to upgrade without thinking about it and have no weirdness that they need to go look at and try to undo. So, I'd really prefer to make this optional and only enabled by default on new site.

cboyden’s picture

What needs to be done to make this optional?

  • Settings option on the panopoly widgets settings page, with the old default (no descriptions) and the new default (yes descriptions)? Or a single checkbox to enable showing descriptions?
  • Update hook to set the setting to its old default on existing sites?
  • Default config to set the initial value to the new default on new installations?
  • Logic in the updated functions to do the right thing according to the setting?
  • Anything else?
dsnopek’s picture

I think the setting should be in the Panopoly Magic settings page. And it could just be a checkbox, but we should use '#states' to only show it when the preview mode is "Single".

But the rest of the points cover it!

cboyden’s picture

OK, here's a patch that provides a checkbox on the Panopoly Magic settings form when single preview mode is enabled. The default is 1, but there's an update hook to set it to 0 if it's not set, which preserves the old behavior. I ran through a few test situations and the settings are doing the right thing, both on a clean install and on update.

I was thinking though - if an existing site is in a mode other than Single preview mode (as is probably the case for most sites created before single preview mode existed), is it OK to let it be enabled with the default of 1? That way if they switch to Single preview mode, they get the default.

One thing I'm not thrilled with is that all of the other form labels are bold. This label is normal font weight. If y'all are OK with that, fine, but if there's a way to render that checkbox so it doesn't get as lost among the other fields, maybe that'd be better.

cboyden’s picture

Assigned: Unassigned » cboyden

Just ran across this issue again after looking more closely at single preview mode. I'll re-roll the patch and see if other updates are needed to make it work.

One more question - does it make sense to also include an option to show the description when in manual preview mode?

cboyden’s picture

Re-rolled patch to apply to latest dev.

cboyden’s picture

Assigned: cboyden » Unassigned
dsnopek’s picture

Status: Needs review » Needs work
FileSize
10.66 KB

Just did some manual testing. Looks good generally, and the styling is nice in Radix, but it doesn't look quite right with responsive_bartik:

Since this will be the new default, we need to make sure it looks nice in responsive_bartik too...

dsnopek’s picture

Status: Needs work » Needs review
FileSize
10.64 KB
4.63 KB

I added a small CSS change that makes it less weird, and has no effect on Radix:

I don't like making style changes like this, because it could have unexpected effects on other themes, but I don't really see a way around it.

  • dsnopek committed 56109e4 on 7.x-1.x
    Update Panopoly Magic for Issue #2611876 by cboyden, dsnopek, mpotter:...
dsnopek’s picture

Status: Needs review » Fixed

I spent a bunch of time thinking about the CSS changes. There's some risk there, but the changes are pretty minimal - it removes more CSS than it adds, and it mostly adds to new markup. The only change that I suspect could be potential disruptive is changing the link text from color: #666 to color: inherit which is not the riskiest CSS change. So, I went for it!

Committed :-)

Status: Fixed » Closed (fixed)

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