After new version released items removed from widget when i click on any place in row (using Adminimal theme). After i am doungraded to 2.0-rc - all is ok, after click on any place in row - it check checkbox.

CommentFileSizeAuthor
#12 interdiff.txt1.18 KBsamvel
#12 entityreference_view_widget-display-empty-message-on-items-removal-2939050-12.patch3.06 KBsamvel
#8 interdiff.txt1.45 KBsamvel
#8 entityreference_view_widget-display-empty-message-on-items-removal-2939050-8.patch3.1 KBsamvel
#7 entityreference_view_widget-display-empty-message-on-items-removal-2939050-7.patch3.14 KBjsacksick
#6 empty_message_after_row_removing-2939050-6.patch3.38 KBsamvel
#4 Selection_016.png53.83 KBsamvel
#4 Selection_015.png11.45 KBsamvel
#2 entityreference_view_widget-remove-item-when-clicking-on-checkbox-only-2939050.patch759 bytesjsacksick

Comments

Samvel created an issue. See original summary.

jsacksick’s picture

jsacksick commented
Status: Active » Fixed
StatusFileSize
new entityreference_view_widget-remove-item-when-clicking-on-checkbox-only-2939050.patch759 bytes

I added a click listener to the checkboxes in the last 2 commits, to remove the row when clicking on the checkbox.
The problem is, the row was also removed when clicking on the label, the attached patch is fixing that.

  • jsacksick committed c4f9d00 on 7.x-2.x 
    Issue #2939050 by jsacksick: Prevent removing rows when clicking on...
samvel’s picture

samvel commented
Status: Fixed » Needs work
StatusFileSize
new Selection_015.png11.45 KB
new Selection_016.png53.83 KB

Please set status fixed issue if it's ok.

I reviewed and see next:
1) There is no empty message when all rows removed. (see screen)
2) Need to change title and description of checkbox in settings "Pass selected entity ids to View" to "Pass all entity ids to View", because there are now new logic and can't be unselected items.

And i think that there are not needed checkboxes anymore (it not work as checkbox) - #2939033-5: Unable to remove item simply

jsacksick’s picture

jsacksick commented

1) true, but won't have time to fix that now
2) I don't think the text is confusing, if you uncheck items, they're gone (i.e non selected).

And i think that there are not needed checkboxes anymore (it not work as checkbox) - #2939033-5

Well, it won't be needed if we have an icon to remove the row, in that case we won't need the checkbox anymore, but it's currently used (When you uncheck it, item is removed, otherwise, it can't be)...

samvel’s picture

samvel commented
Status: Needs work » Needs review
StatusFileSize
new empty_message_after_row_removing-2939050-6.patch3.38 KB

1) Patch attached
Also fixed typo errors and one javascript error - there are in behavior was code with triggers (click), but without .once - it's cause multiple execution of trigger. I fixed it for widget part (not for modal), because it's prevented me doing this issue.
For modal - may be need review code if there are same errors - but it's another issue.

2) Will be done in #2939033: Unable to remove item simply

jsacksick’s picture

jsacksick commented
StatusFileSize
new entityreference_view_widget-display-empty-message-on-items-removal-2939050-7.patch3.14 KB

I simplified the JS code, there's no need to perform an each loop now that we added "once", please let me know if the attached patch works as expected on your side as well.

samvel’s picture

samvel commented
StatusFileSize
new entityreference_view_widget-display-empty-message-on-items-removal-2939050-8.patch3.1 KB
new interdiff.txt1.45 KB

Hi! It looks simpler and work for me.

Here few moments from me:
1) Why we need :first in var $table = $row.parents('table.field-multiple-table:first'); In what case we have few tables with that class? I not changed it.

2) I replaced also one place with $row

3) I suggest not set empty row when message empty (it wil lbe possible if empty text will be moved in future to settings form), because it will look bad if table have any borders...

See patch and interdiff (with #7).

jsacksick’s picture

jsacksick commented 
+++ b/entityreference_view_widget.js
@@ -21,14 +17,15 @@
+          if (!$row.is(':checked')) {

This doesn't look right to me... $row is the

, whereas I was targeting the checkbox before...

So are you sure this is functional?

jsacksick’s picture

jsacksick commented
Status: Needs review » Needs work
jsacksick’s picture

jsacksick commented

1) Why we need :first in var $table = $row.parents('table.field-multiple-table:first'); In what case we have few tables with that class? I not changed it.

We could probably remove the :first and change to parent, this needs to be tested.

samvel’s picture

samvel commented
StatusFileSize
new entityreference_view_widget-display-empty-message-on-items-removal-2939050-12.patch3.06 KB
new interdiff.txt1.18 KB

This doesn't look right to me... $row is the

, whereas I was targeting the checkbox before...

Oh yes, sorry, my mistake.

We could probably remove the :first and change to parent, this needs to be tested.

Now i understand, it's ok.

patch and interdiff (with #7)

samvel’s picture

samvel commented
Status: Needs work » Needs review

  • jsacksick committed 4ead334 on 7.x-2.x authored by Samvel 
    Issue #2939050 by Samvel, jsacksick: Display an empty message when all...
jsacksick’s picture

jsacksick commented
Status: Needs review » Fixed

Committed.

Status: Fixed » Closed (fixed)

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