And from that I guess... consensus has been reached, before another patch was thrown ? ;)
@heddn:
#85
π
Allow for deletion of a single value of a multiple value field
Fixed
is the last
comprehensive patch on the AJAX path, but as mentioned in
#89
π
Allow for deletion of a single value of a multiple value field
Fixed
it has some problems.
There's a lot of noise - people learning how to make patches, incomplete solutions, re-rolls for obsolete versions, #143 π Allow for deletion of a single value of a multiple value field Fixed is just a last of "works for my limited use-case" patches (only textfield - started in #128 π Allow for deletion of a single value of a multiple value field Fixed ).
Looking into it I was a little bit surprised it didn't depend on core's 'RESTful Web Services' and instead used custom controller with JsonResponse()
.
So the question I have (which is not framework specific, but since it is the first one to be implemented and the Polymer one is based on it) is what are the challenges of making it use things like:
1. dblog's @RestResource
(core\modules\dblog\src\Plugin\rest\resource\DBLogResource.php
), or @RestResource
in general since the one in core is very basic.
2. REST export
display type in Views
to show tighter integration with Drupal ?
Re-roll of #131 after #2898261: In jQuery 3, $('#') throws syntax errors β
Should be fixed now.
There's still some JS double-click issue when used within Paragraphs.
Edit: Caused by nested '.field-multiple-table'
tables.
@samuel.seide: Here it is, also updated for ES6.
@samuel.seide: Thanks for testing, it happens because the code assumes (wrongly) that each field extending WidgetBase
and overwriting ::formMultipleElements()
will call parent::formMultipleElements()
, but Paragraphs doesn't.
I will post an updated patch in a couple of days.
I'm not familiar with Paragraphs, is your expectation it will somehow 'work' with it, or just not break ?
@rgpublic: I didn't, frankly they shouldn't use it on production, it's a hard problem, the issue was created in 2011 and is slow enough even without it, the proposed approach doesn't even have blessing of the system maintainers yet.
What would be very helpful is tests ;) they would be useful even if approach changes.
Could you please stop this nonsense of rolling for 8.3.x/8.2.x ?
It is just pointless noise, there is no way this gets into 8.3.x.
Minor CSS cleanup, swapping order of button and checkbox to get styles consistent with file/image widget.
No-js graceful degradation and coding standard fixes from #112, left out "apply the remove button when the field is not a new/blank field item" as it forces you to "Add another item" to be able to delete last one, it makes sense in file/image widgets due to their nature but not on generic fields in my opinion.
Still no tests,
Also needs work for fields with fixed (>1) cardinality, and no-js graceful degradation.
@bleen: yes the alternative approach solves all of them by not messing with $form_state
over AJAX.
Missed .once()
in behavior, converted to button triggered to be consistent with field and image widgets.
Adds class 'removed' to the row so it could be themed or just "display: none"d.
@rgpublic: it is added inside if ($is_multiple) {...}
right beside '_weight'
so it really shouldn't
How about something simpler ?
Adding column and a checkbox, that is used in WidgetBase::extractFormValues()
to filter out unwanted values.
The advantage is that it avoids AJAX callbacks, also you can change your mind and just un-check.
Could also add JS/behavior for it that hides the checkboxes and adds buttons in their place (that just toggle the hidden checkboxes).