OptGroups::flattenOptions() currently returns an array where only keys are relevant, and values are useless (all '1').

OptionsWidgets & OptionsFormatter need a "flatten" operation that preserves labels - thus they each need to include their own duplicate version (which currently does not support the "object" syntax for optgroups).

OptGroups::flattenOptions() could simply preserve the labels instead of putting a useless '1' value ?

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task : expand functionality of a Form helper method to allow code reuse
Issue priority Not critical : if not done, just means code duplictaion in Option widgets and formatters ...
Disruption None, the array values returned by the method were not meaningful so far (all '1') and were thus not used by calling code, that only used the array keys. Patch makes the return values useful, existing calling code is not affected

Comments

yched’s picture

Status: Active » Needs review
StatusFileSize
new467 bytes

Could it be that simple ?

yched’s picture

Assigned: Unassigned » chx

Wow - that code dates from #39179: Make optgroups work with choice checker from 2006... (the object syntax was added more recently)

Probably means this should be vetted by @chx :-)

dawehner’s picture

Status: Needs review » Needs work
Issue tags: +Ghent DA sprint

Its interesting, but yeah the test itself will certainly fail here, it expects this exact behaviour.

The last submitted patch, 1: 2392301-OptGroup_values-1.patch, failed testing.

chx’s picture

Assigned: chx » Unassigned

I really have a hard time imagining anything breaking from this (except from the test...). Good grief, 2006...

joelpittet’s picture

Status: Needs work » Needs review
StatusFileSize
new1.33 KB

Would this be good enough or do we need to deal with the test more?

Unsure about the delete, but it's the only provider value that has a value that isn't 'foo' when flattened.

yched’s picture

@joelpittet: yay, thanks !

@chx, RTBC then ? :-)

tstoeckler’s picture

Status: Needs review » Reviewed & tested by the community

Yeah I think that means RTBC. ...I support the motion :-)

yched’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new4.95 KB
new3.62 KB

Now that #2238085: [regression] options_allowed_values() signature doesn't allow for Views filter configuration got in, let's actually put this to use and remove the duplicate helpers in OptionsWidgetBase & OptionsDefaultFormatter.

chx’s picture

Status: Needs review » Reviewed & tested by the community

Yes, done.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs issue summary update

This issue is a normal task so we need to outline how it fits within the allowable Drupal 8 beta criteria. Can someone add Drupal 8 beta phase evaluation template to the issue summary.

yched’s picture

Issue summary: View changes
Status: Needs work » Reviewed & tested by the community

Added the beta phase evaluation template

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

I like less duplicate code - it makes Drupal less fragile - committing this under the beta fragility clause. Committed dbc572c and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation for to the issue summary.

  • alexpott committed dbc572c on 8.0.x
    Issue #2392301 by yched, joelpittet: OptGroups::flattenOptions() should...

Status: Fixed » Closed (fixed)

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