We should be able to sort the FieldItemList without converting back and forth to an array.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yched’s picture

Title: Stop the getValue() / setValue() danse in WidgetBase::sortItems() » Add ItemList::sortItems($callback) to remove the getValue() / setValue() danse in WidgetBase::sortItems()
Assigned: yched » Unassigned
Status: Active » Needs review
Issue tags: +Entity Field API
FileSize
1.69 KB

Since we can't do a usort on a FieldItemList, then the FieldItemList needs to be able to do it for us...

yched’s picture

Title: Add ItemList::sortItems($callback) to remove the getValue() / setValue() danse in WidgetBase::sortItems() » remove the getValue() / setValue() danse in WidgetBase::sortItems()
FileSize
3.71 KB

Actually - there might be a case for sorting abilities on (Field)ItemList, but in our specific case here, it makes more sense to sort the raw form values before they get assigned to the field. Cleaner code in WidgetBase::extractFormValues().

Status: Needs review » Needs work

The last submitted patch, 2: widgetBase_sortItems-2170979-3.patch, failed testing.

The last submitted patch, 2: widgetBase_sortItems-2170979-3.patch, failed testing.

yched’s picture

Status: Needs work » Needs review
FileSize
6.21 KB
4.33 KB

Multiple values widgets needed more care.
We should try to revive #1846162: Cleanup Widgets API : add a separate base class for 'multiple' widgets, but meanwhile this should do it.

yched’s picture

Title: remove the getValue() / setValue() danse in WidgetBase::sortItems() » remove the getValue() / setValue() dance in WidgetBase::sortItems()

"danse" is UK, right ? or just plain wrong ?

swentel’s picture

swentel’s picture

Status: Needs review » Reviewed & tested by the community

Nice cleanup. Asked for a retest just to be sure, RTBC when it comes back green.

yched’s picture

yched’s picture

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Nice tidy up - Committed a59a468 and pushed to 8.x. Thanks!

yched’s picture

Status: Fixed » Reviewed & tested by the community
FileSize
510 bytes

Sorry, quick minor followup - don't know what got into me, that added "return" is totally useless.
Trivial, setting straight back to RTBC. Spank me :-)

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 85a75d4 and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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