Here's a collection of small changes I made to work with ctools & panels in D7.
I tried to group the changes by topic / file to make it easier to apply and review.

ctools-context-admin-form-definition:

  • Added several keys to form specific arrays to get rid of "Undefined index" notices
  • Removed unnecessary variable definitions ($position)
  • ctools_context_get_defaults needs review. Removed constructs that looked legacy
  • Removed trailing space


ctools-context-inc:

  • Added $plugin as parameter in some cases. Some of the function seem to rely on this information - but I can't remember which one exactly.
  • Create a more specific name for converters.
  • Use entity:user instead user to load user context.
  • Added additional check for $access['logic'] to make sure theres no "Undefined index" notice - needs review


ctools-context-theme-inc:

  • Prevent "Undefined index" notice. But I'm actually not sure if is / should be description optional.


ctools-field-inc:


ctools-notices-fixes:


ctools-vocabularies-and-terms:

  • Changed naming to be entity compliant.


ctools-views_panes-smarter-title:

  • Better title for views_panes.
CommentFileSizeAuthor
#6 ctools-notices-fixes-1014866-6.patch1.36 KBdas-peter
#6 ctools-plugins-admin-inc-1014866-6.patch532 bytesdas-peter
#4 ctools-views_panes-smarter-title.patch623 bytesdas-peter
ctools-context-admin-form-definition.patch2.48 KBdas-peter
ctools-vocabularies-and-terms.patch1.87 KBdas-peter
ctools-views_panes-smarter-title.patch707 bytesdas-peter
ctools-plugins-admin-inc.patch1.04 KBdas-peter
ctools-notices-fixes.patch3.19 KBdas-peter
ctools-field-inc.patch1.96 KBdas-peter
ctools-context-theme-inc.patch506 bytesdas-peter
ctools-context-inc.patch3.03 KBdas-peter

Comments

merlinofchaos’s picture

merlinofchaos commented

One review. Still working.

diff --git views_content/plugins/content_types/views_panes.inc views_content/plugins/content_types/views_panes.inc
index 894c263..09b5897 100644
--- views_content/plugins/content_types/views_panes.inc
+++ views_content/plugins/content_types/views_panes.inc
@@ -90,7 +90,13 @@ function _views_content_panes_content_type($view, $display) {
 
   $title = $display->handler->get_option('pane_title');
   if (!$title) {
-    $title = $view->name;
+    $display_title = $display->display_title;
+    if (!$display_title) {
+      $title = $view->name;
+    }
+    else {
+      $title = $display_title . ' [' . $view->name . ']';
+    }
   }
 
   $description = $display->handler->get_option('pane_description');

Should be a translated title. Something like t('%display [%view]'). It should probably also be consistent with how views are listed in the block UI in the same circumstance.

merlinofchaos’s picture

merlinofchaos commented 

@@ -594,8 +608,8 @@ function ctools_context_ajax_item_edit($mechanism = NULL, $type = NULL, $cache_k
 
 function ctools_context_get_defaults($plugin_definition) {
   $conf = array(
-    'identifier' => $plugin_definition['title'] . ($id > 1 ? ' ' . $id : ''),
-    'keyword' => ctools_get_keyword($object, $plugin_definition['keyword']),
+    'identifier' => $plugin_definition['title'],
+    'keyword' => $plugin_definition['keyword'],
     'name' => $plugin_definition['name'],
   );

I don't see how this can be right? Am I missing something?

eclipsegc’s picture

eclipsegc commented

A lot of these look really great, but with this many changes, I'd much rather a handful of small patches need a little revisiting than re-rolling a 750 line patch regularly and often. Can I side track this train into getting some review on #951048: Support Subtypes for Arguments, Context and Relationship plugins before we go altering large sections of the code base again?

My patch is altering:

includes/content.menu.inc
includes/context.inc
includes/context.plugin-type.inc
plugins/access/term.inc
plugins/access/term_parent.inc
plugins/access/term_vocabulary.inc
plugins/arguments/entity_id.inc

The rest are new files.

I think specifically I'd like to see a hold on:

ctools-context-inc.patch
ctools-vocabularies-and-terms.patch

The rest are probably fine.

Eclipse

das-peter’s picture

das-peter commented
StatusFileSize
new ctools-views_panes-smarter-title.patch623 bytes

Thank you guys for your feedback.

#1:
Changed it to how it's done in views_plugin_display_block.inc execute_hook_block_list().

#2:
Well, my approach was very lazy. There's no $id nor $object - thus remove the constructs.
Does anything obviously fail - if not = done.
Thus it's very likely that this isn't properly fixed - and if someone can give my a direction I'll fix it.
Btw. this seems the last place where ctools_get_keyword() was used.

#3:
Let's skip the patches here touched by #951048: Support Subtypes for Arguments, Context and Relationship plugins. I'll keep an eye on it and make later a reroll where necessary.

merlinofchaos’s picture

merlinofchaos commented
Status: Needs review » Needs work

Committed smarter title panes and context-theme.inc Marking the rest needs work.

das-peter’s picture

das-peter commented
StatusFileSize
new ctools-plugins-admin-inc-1014866-6.patch532 bytes
new ctools-notices-fixes-1014866-6.patch1.36 KB

Patches cleaned:
ctools-plugins-admin-inc: Removed patch duplication
ctools-notices-fixes: Remove duplication of ctools-plugins-admin-inc.

merlinofchaos’s picture

merlinofchaos commented
Status: Needs work » Needs review

Resetting status

merlinofchaos’s picture

merlinofchaos commented
Status: Needs review » Fixed

Committed. I know there are more mostly related to entities; let's follow them up in separate issues as I'm having trouble tracking everything here.

Status: Fixed » Closed (fixed)

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

pillarsdotnet’s picture

pillarsdotnet commented

See related: #1241170: Undefined variable $id in ctools_context_get_defaults() (line 619 of includes/context-admin.inc)