Extract DraggableListBuilderTrait from DraggableListBuilder for use with content entities

Converted the patch into a MR now, with #17 as the initial commit so my changes are easily reviewable. Went ahead and implemented #8 now with a tiny change in EntityListBuilder (to set $this->formBuilder) (but not changes required in any sub-classes). And also fixed #26. Verified that everything still works and also verified that adding more property or return type hints causes breakage because that would require sub-classes to also specify those.

Seems points #26 have been addressed in MR 5154. Since this was previously RTBC before that going to restore

Started leaving comments about adding more type hints on the MR, then I realised I should have read #29 properly, of course it will affect subclasses.

Committed a993304 and pushed to 11.x (which will also become 10.3.x). Thanks!

catch → closed merge request !5154

I was going to file a follow-up for the injection of formBuilder, but 📌 Replace non-test usages of \Drupal::formBuilder() with IoC injection Needs work should cover it.

It missing change notice

The commit from #33 expects a return value int|float for the getWeight method in DraggableListBuilder. I just came across a config entity, that has a weight property, but that may be NULL. In that case, this throws an exception.

Should the getWeight method maybe fall back to 0, if the weight property has no value?

Re #35: fair enough, makes sense. Added a draft change notice: https://www.drupal.org/node/3425054 → . Feel free to publish if you think it's alright. (And then ideally set the issue back to fixed)

Re #37: Wow, never thought of that possibility, but yeah, totally makes sense to not blow up in that case, doesn't really cost much to add that fallback: Opened 🐛 [regression] Add a fallback in DraggableListBuilder::getWeight() to support entities with a NULL weight Needs review Sorry for the trouble and thanks for reporting it.

Marking RTBC for the CR. Would like a committer to also see before publishing it.

CR looks great. Publishing.

> The commit from #33 expects a return value int|float for the getWeight method in DraggableListBuilder. I just came across a config entity, that has a weight property, but that may be NULL. In that case, this throws an exception.

Similar issue -- I've just tried to use DraggableListBuilder on a config entity, and got a crash because the weight is a string not an int.

Follow-up -- and I wonder whether we should fix this rather than document it: 🐛 DraggableListBuilder / DraggableListBuilderTrait need to document that buildRow() behaves completely differently from the parent class Active .

Re #41: Interesting, that didn't come up here because that would be fixed as soon as you add config schema to your entity type. But I guess it wouldn't hurt to add a (float) type-cast to getWeight(). I can open a follow-up for that, if people think that makes sense.

Ah yeah, the bug is that environment_indicator has declared this incorrectly:

    weight:
      type: string
      label: 'Name'

> But I guess it wouldn't hurt to add a (float) type-cast to getWeight(). I can open a follow-up for that, if people think that makes sense.

On second thoughts, we shouldn't babysit that, but we should instead add something to the trait docs to say the weight field must be an int.

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