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 ?
| 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 |
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | interdiff.txt | 3.62 KB | yched |
| #9 | 2392301-OptGroups_values-9.patch | 4.95 KB | yched |
| #6 | 2392301-6.patch | 1.33 KB | joelpittet |
| #1 | 2392301-OptGroup_values-1.patch | 467 bytes | yched |
Comments
Comment #1
yched commentedCould it be that simple ?
Comment #2
yched commentedWow - 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 :-)
Comment #3
dawehnerIts interesting, but yeah the test itself will certainly fail here, it expects this exact behaviour.
Comment #5
chx commentedI really have a hard time imagining anything breaking from this (except from the test...). Good grief, 2006...
Comment #6
joelpittetWould 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.
Comment #7
yched commented@joelpittet: yay, thanks !
@chx, RTBC then ? :-)
Comment #8
tstoecklerYeah I think that means RTBC. ...I support the motion :-)
Comment #9
yched commentedNow 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.
Comment #10
chx commentedYes, done.
Comment #11
alexpottThis 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.
Comment #12
yched commentedAdded the beta phase evaluation template
Comment #13
alexpottI 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.