Love the module, simple and much needed. However there doesn't seem to be Views support? I'm offering an exposed field to the users in Views and I would like the taxonomy selection list to have the nice format this module provides. Is this possible?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

-Mania-’s picture

+1

Chi’s picture

Title: Views support » Support optgroups for taxonomy selection list
Project: Taxonomy container » Views (for Drupal 7)
Version: 7.x-1.1 » 7.x-3.x-dev
Component: Code » exposed filters
Status: Active » Needs review
FileSize
47.94 KB
27.75 KB
2.31 KB

I think it should be fixed in views module.

Settings

Dropdown

dawehner’s picture

Status: Needs review » Needs work
+++ b/modules/taxonomy/views_handler_filter_term_node_tid.incundefined
@@ -41,6 +41,7 @@ class views_handler_filter_term_node_tid extends views_handler_filter_many_to_on
+    $options['groups'] = array('default' => 0);

It would be cool to name the internal key "optgroups", because this, at least for me, makes it much easier to understand.

Chi’s picture

Status: Needs work » Needs review
FileSize
2.33 KB
dawehner’s picture

Thanks for the change! From the technical perspective this patch is perfect, the only question i have is whether this should be actually implemented in views itself or in taxonomy container.

I will ask some people about their oppinions.

philipz’s picture

This option is just what I was looking for and the patch is simple and it just works. One question. Do we need str_repeat('-', $term->depth) in the 'Show groups in dropdown' option? This is no longer needed I think.

Chi’s picture

This is no longer needed I think.

Why not? Optgroups does not override hierarchy in dropdown list.

philipz’s picture

When you add optgroups you add grouping to options which means hierarchy. It looks a bit different in major browsers but adding '-' in front of terms/options that are inside optgroups seems obsolete. The dashes are there only to visually imitate the hierarchy.

EDIT: OK now I see that the optgroups are only for the top terms aren't they? If so maybe there could be one dash less for every option level to make it look better?

str_repeat('-', $term->depth-1)

Chi’s picture

Yep, it looks better. Removed one dash on every option as suggested.

Chi’s picture

Minor code style fix

semiaddict’s picture

#10 works great.
Thank you.

semiaddict’s picture

I ended up having to make a small change on line 51 of the patch to make it work when the optgroup option is not selected, otherwise I got the following error:
Warning: str_repeat() [function.str-repeat]: Second argument has to be greater than or equal to 0 in views_handler_filter_term_node_tid->value_form() (line 147 of ../sites/all/modules/views/modules/taxonomy/views_handler_filter_term_node_tid.inc).

The modified patch is attached.

giufog’s picture

how and where do I install it?

Ante890’s picture

Issue summary: View changes

Time to commit it ?

nimek’s picture

+1 for commit very usefull functionality

Lang Wu Jane’s picture

Can any one help to commit this functionality? Please
By the way, this patch works fine for me, but it seems that the configuration for optgroup couldn't be exported...

Anonymous’s picture

Why isn't the views_optgroups-1695658-11.patch being committed within views? Views 3.13 was just released but I didn't see it.

I see Jane_wu's comment that it couldn't be exported; unfortunately I'm not familiar with the process. Any answers?

Ante890’s picture

Ante890’s picture

Status: Needs review » Reviewed & tested by the community
jzavrl’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
2.5 KB

This is not OK yet. When using the latest patch the localised terms are not being used. If you take a look at the previous implementation:

$choice->option = array($term->tid => str_repeat('-', $term->depth) . entity_label('taxonomy_term', $term));

It's using entity_label() which provides the localised term. I'm reattaching the new patch using this implementation and now the correct term label is being shown based on the current language.

Status: Needs review » Needs work

The last submitted patch, 21: views_optgroups-1695658-21.patch, failed testing.

jzavrl’s picture

Status: Needs work » Needs review
FileSize
2.54 KB

Reattached a freshly generated patch.

Status: Needs review » Needs work

The last submitted patch, 23: views_optgroups-1695658-22.patch, failed testing.

jespermb’s picture

Status: Needs work » Needs review
FileSize
2.5 KB

I tested the patch and found a few issues which I have corrected.

aloyr’s picture

Status: Needs review » Reviewed & tested by the community

Patch#1695658-25 still works for me.

coconnor’s picture

#25 does the trick :)

millerrs’s picture

#25 also works for me. Thanks!

dmitryl’s picture

views_optgroups-1695658-25.patch works. Thanks!

travisc’s picture

Also tested and looks good, RTBC.

glynster’s picture

+1 RTBC please!

Ryan Osītis’s picture

Anyone know why this hasn't been committed? It works well and should have been included in the 7.x-3.19 release. Anyone know if we need to re-roll the patch in #25?

glynster’s picture

Manually applied and works well as before please RTBC!

aloyr’s picture

Patch #25 works, but doesn't apply cleanly.

Updating it to run on git ref 481b5c28 (current for 7.x-3.x branch as of 2018-04-19).

millerrs’s picture

#34 works for me.

DamienMcKenna’s picture

Status: Reviewed & tested by the community » Needs work
Parent issue: » #2960871: Plan for Views 7.x-3.23 release

A minor request - could you please revert the change so that the $tree assignment is on a separate line, instead of nested inside an if() statement? Thanks.

aloyr’s picture

ok, done. :) now that assignment is on its own line.

aloyr’s picture

Status: Needs work » Needs review
millerrs’s picture

I tested #37 and it works.

millerrs’s picture

Status: Needs review » Reviewed & tested by the community
Ryan Osītis’s picture

I tested #37 and it's a winner. Works great.

Simon Georges’s picture

For those who may need it, the same patch for core 8.6.x, which seems to work exactly the same way.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 42: views-1695658-42-patch-for-core-8.x.patch, failed testing. View results

Chi’s picture

Project: Views (for Drupal 7) » Drupal core
Version: 7.x-3.x-dev » 8.7.x-dev
Component: exposed filters » views.module
Status: Needs work » Needs review

I don't think this will ever be fixed in Views - 3.x. Moving to core issue queue since the Views module is a part of Drupal 8 now.

Simon Georges’s picture

Just in case it ever get committed, all credits for this patch goes not to me, but to precedent authors, I merely copy-pasted it in the right (it seems) Drupal 8 core file.

Simon Georges’s picture

Status: Needs review » Needs work

The last submitted patch, 46: core-1695658-46-opt_group_support.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

DamienMcKenna’s picture

Project: Drupal core » Views (for Drupal 7)
Version: 8.7.x-dev » 7.x-3.x-dev
Component: views.module » Code
Status: Needs work » Needs review

Please create a new issue and relate this one to it for the core changes.

Moving this back to Views for D7.

DamienMcKenna’s picture

Rerolled, plus a small tweak.

  • DamienMcKenna committed a19d7d8 on 7.x-3.x authored by Chi
    Issue #1695658 by Chi, aloyr, Simon Georges, jzavrl, DamienMcKenna,...
DamienMcKenna’s picture

Status: Needs review » Fixed

Committed. Thanks!

Chi’s picture

Status: Fixed » Closed (fixed)

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

m.stenta’s picture

This change causes the following PHP notices and warnings to appear:

    Notice: Undefined variable: options in views_handler_filter_term_node_tid->value_form() (line 237 of /var/www/html/sites/all/modules/git/views/modules/taxonomy/views_handler_filter_term_node_tid.inc).
    Notice: Undefined variable: options in views_handler_filter_term_node_tid->value_form() (line 238 of /var/www/html/sites/all/modules/git/views/modules/taxonomy/views_handler_filter_term_node_tid.inc).
    Warning: Invalid argument supplied for foreach() in views_handler_filter->prepare_filter_select_options() (line 1208 of /var/www/html/sites/all/modules/git/views/handlers/views_handler_filter.inc).
    Warning: Invalid argument supplied for foreach() in form_select_options() (line 2841 of /var/www/html/includes/form.inc).

Looking at line 237 of views_handler_filter_term_node_tid.inc in PHPStorm, the IDE warns that "Variable $options might have not been defined". It seems as if this change added conditionals that under certain conditions result in the variable not being created.

In addition to that, on line 165 the variable $parent_name is used without being defined as well (this appears as a red underline in PHPStorm, which is more severe than the "might have not been defined" on line 237.

m.stenta’s picture

Why was line 148 removed in this commit?

https://git.drupalcode.org/project/views/commit/a19d7d8#a8d18f38f8f4cfe0...

That is where $options was initialized:

        $options = array();
m.stenta’s picture

Why was line 148 removed in this commit?

It appears that the patch in comment #19 above is where this line was removed - without explanation. And that went undetected.

I will open a new issue and provide a patch to restore that line.

m.stenta’s picture

m.stenta’s picture

I still don't know what to do about the undefined $parent_name on line 165: https://git.drupalcode.org/project/views/blob/2c4f62ade0af1a2541b1bd1a92...

@Chi - can you take a look at that? Maybe you can see how to fix it?

Chi’s picture

That patch was originally based on the code from Taxonomy container module. PhpStorm complains on $parent_name because that variable is defined conditionally. However, it does not cause any PHP errors. When PHP accesses that variable it is always defined because of the way terms are sorted.
You can tell PhpStorm about that fact with the following notation.
/** @var \Drupal\taxonomy\TermInterface $parent_name */

m.stenta’s picture

@Chi - Oh I see how it works now - thanks for the explanation. Looks good! So I think the only thing that needs to be fixed is #3065777: Notice: Undefined variable: options in views_handler_filter_term_node_tid->value_form()