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

CommentFileSizeAuthor
#12 quick_followup-2170979-12.patch510 bytesyched
#10 widgetBase_sortItems-2170979-10.patch6.21 KByched
#5 interdiff.txt4.33 KByched
#5 widgetBase_sortItems-2170979-5.patch6.21 KByched
#2 widgetBase_sortItems-2170979-3.patch3.71 KByched
#1 widgetBase_sortItems-2170979-1.patch1.69 KByched

Comments

yched’s picture

yched commented
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
StatusFileSize
new widgetBase_sortItems-2170979-1.patch1.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

yched commented
Title: Add ItemList::sortItems($callback) to remove the getValue() / setValue() danse in WidgetBase::sortItems() » remove the getValue() / setValue() danse in WidgetBase::sortItems()
StatusFileSize
new widgetBase_sortItems-2170979-3.patch3.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

yched commented
Status: Needs work » Needs review
StatusFileSize
new widgetBase_sortItems-2170979-5.patch6.21 KB
new interdiff.txt4.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

yched commented
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 commented

5: widgetBase_sortItems-2170979-5.patch queued for re-testing.

swentel’s picture

swentel commented
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 commented

5: widgetBase_sortItems-2170979-5.patch queued for re-testing.

yched’s picture

yched commented
StatusFileSize
new widgetBase_sortItems-2170979-10.patch6.21 KB

Reroll after #2141539: Rename FieldItemListInterface::filterEmptyValues() to filterEmptyItems()

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

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

yched’s picture

yched commented
Status: Fixed » Reviewed & tested by the community
StatusFileSize
new quick_followup-2170979-12.patch510 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

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
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.