Bug 38020

Created attachment 173307 [details] [review]
Bug 38020: Change form to submit link

To test:

1. Create a list in the staff interface
2. Go to lists
3. View the list
4. Click the Edit dropdown
5. Notice the two dropdown options for Edit list and Delete list have different formatting and look different
6. Apply patch
7. Try steps 1 through 4 again
8. The delete list button form looks right, and delete function works

Created attachment 173311 [details] [review]
Bug 38020: Change form to submit link

To test:

1. Create a list in the staff interface
2. Go to lists
3. View the list
4. Click the Edit dropdown
5. Notice the two dropdown options for Edit list and Delete list have different formatting and look different
6. Apply patch
7. Try steps 1 through 4 again
8. The delete list button form looks right, and delete function works

Signed-off-by: David Nind <david@davidnind.com>

FYI for anyone trying to solve similar problems:

A control in a dropdown menu that looks like this:

<button type="submit" class="btn btn-default delete-list">Delete list</button>

...will look wrong because of the "btn btn-default" class. If you change it to "dropdown-item" it looks fine without making other markup changes.

<button type="submit" class="dropdown-item delete-list">Delete list</button>

There's no guideline saying one is better than the other so it makes no difference for this patch.

I don't think this is PQA ready but don't think the reasons below are enough to FQA either, so I'm only leaving comments here in case they make sense:

1) The following code:

  $(".delete-list").click(function(e){
    return confirm(_("Are you sure you want to delete this list?"));
  });

in virtualshelves/shelves.tt is no longer used after this patch, because the confirmation message is now part of the anchor link. If this is true, it should be removed.


2) Not related to this patch directly, but related to the previous point, virtualshelves/tables/shelves_results.tt has 'class="delete-list' for the delete button on the list view, but the JS code in 1) is only used on the 'view' op, not the 'list' op, thus deleting a list from the detail page has a confirmation dialog, but deleting a list from the list view does not have a confirmation dialog.

Don't be scared to set Failed QA for questions and clarification - it's not a bad thing and the status change helps to get attention as it will show on the personal dashboard for example, while an email is easy to miss. We need to make that less scary maybe :)

(In reply to Katrin Fischer from comment #5)
> Don't be scared to set Failed QA for questions and clarification - it's not
> a bad thing and the status change helps to get attention as it will show on
> the personal dashboard for example, while an email is easy to miss. We need
> to make that less scary maybe :)

Agreed. Thanks!

Created attachment 173724 [details] [review]
Bug 38020: Change form to submit link

To test:

1. Create a list in the staff interface
2. Go to lists
3. View the list
4. Click the Edit dropdown
5. Notice the two dropdown options for Edit list and Delete list have different formatting and look different
6. Apply patch
7. Try steps 1 through 4 again
8. The delete list button form looks right, and delete function works

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 173725 [details] [review]
Bug 38020: (follow-up) Remove unused delete confirmation code

This follow-up patch removes the unused delete list confirmation dialog in virtualshelves/shelves.tt

(In reply to Pedro Amorim from comment #4)
> 
> 1) The following code:
> 
>   $(".delete-list").click(function(e){
>     return confirm(_("Are you sure you want to delete this list?"));
>   });
> 
> in virtualshelves/shelves.tt is no longer used after this patch, because the
> confirmation message is now part of the anchor link. If this is true, it
> should be removed.
> 
> 
> 2) Not related to this patch directly, but related to the previous point,
> virtualshelves/tables/shelves_results.tt has 'class="delete-list' for the
> delete button on the list view, but the JS code in 1) is only used on the
> 'view' op, not the 'list' op, thus deleting a list from the detail page has
> a confirmation dialog, but deleting a list from the list view does not have
> a confirmation dialog.

Thank you for your comment! 
I added a follow-up for 1) also opened a ticket for 2) Bug 38302

Created attachment 173747 [details] [review]
Bug 38020: Change form to submit link

To test:

1. Create a list in the staff interface
2. Go to lists
3. View the list
4. Click the Edit dropdown
5. Notice the two dropdown options for Edit list and Delete list have different formatting and look different
6. Apply patch
7. Try steps 1 through 4 again
8. The delete list button form looks right, and delete function works

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>

Created attachment 173748 [details] [review]
Bug 38020: (follow-up) Remove unused delete confirmation code

This follow-up patch removes the unused delete list confirmation dialog in virtualshelves/shelves.tt

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>

Created attachment 173749 [details] [review]
Bug 38020: (QA follow-up) Decapitilise 'list'

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>

Pushed for 24.11!

Well done everyone, thank you!