CSS based deletion confirmation, #295

[daonb]

Status

Work in progress. Just index.html was updated, I will change col.html if this way is accepted.
I'm also not sure about the colors, should Delete be blue and Confirm red or both of them red?

Description of Changes

Fixes #295

Changes proposed in this pull request: Delete confirmation is based on CSS and does not require JS

Testing

Send a message as a whistleblower, log as a journalist mark the message and press Delete. A confirm button will appear next to the delete button.

[ghost]

@daonb please ping me explicitly with @dachary once you have resolved the errors (GitHub won't let me know that you did ;-).

[heartsucker]

Ooooh snap. This is clean.

I would change the button to say "confirm delete" so it's very obvious what's happening. This will also future proof it if we choose to add other confirmations to other actions in the future.

[heartsucker]

You also have a linting error:

html_lint.py --printfilename --disable=optional_tag,extra_whitespace,indentation \
		securedrop/source_templates/*.html securedrop/journalist_templates/*.html
Traceback (most recent call last):
  File "/usr/local/bin/html_lint.py", line 66, in <module>
    sys.exit(html_linter.main(options))
  File "/usr/local/lib/python2.7/dist-packages/html_linter.py", line 1094, in main
    clean_html = template_remover.clean(io.open(filename).read())
  File "/usr/local/lib/python2.7/dist-packages/template_remover.py", line 258, in clean
    return _TemplateRemover(html_content, pattern).get_clean_content()
  File "/usr/local/lib/python2.7/dist-packages/template_remover.py", line 199, in get_clean_content
    assert False, 'Echo tags should be in just one line.'
AssertionError: Echo tags should be in just one line.

You can run the linters in the dev machine from the SD root with make lint or just make html-lint since you only changed the HTML.

[heartsucker]

I also think it's very likely this will break the functional tests with Selenium since it will expect the click on "Delete" to send the request.

[daonb]

@heartsucker seems like making it a Confirm Delete breaks the line and we're not sure it's worth it.

[heartsucker]

@daonb what you could do is wrap the pair in its own div so that the latter appears below it and apply some styling. I'm not great with HTML / CSS, but something like:

<div class="confirm-container">
  <button>Foo Action</button>
  <button>Foo Confirm</button>
</div>
...

and then force it to wrap below.

[huertanix]

A quick design note: Having a confirmation button alongside top-level options seems somewhat dissimilar to the sort of confirmation dialogs that many users may be used to. I would recommend having the confirm button live inside a second row that can appear under the top-level options row with a light-red background with some copy asking the user to confirm or cancel:

"Are you sure you want to delete the selected [singular or plural whatever word we use to describe what is being deleted]?" [Yes] [Cancel]

[ghost]

@daonb I volunteer to address the comments in case you don't have time right now. Just let me know ;-)

[daonb]

@dachary Thanks for the offer. We have another meeting the coming Monday and I hope to finish it than. If I don't, it'll be all yours :-)

[daonb]

@dachary it didn't happen :-( . what's more, that crooked apple have talked me into high-sierra which means I now have to download the box which takes over 2 hours and I only got an hour in this cafe before I sign off.
If you got time to complete this, I'd be happy if you do.

[ghost]

@daonb :-) I'll do it, sure.

[ghost]

@toast this is a draft to implement deletion confirmation without javascript. As @huertanix mentions, the proposed UX is somewhat unusual. Do you have an idea on how to make it better ? I can work on the implemenation based on a mockup, following the best practices you described when working on the i18n menu.

[ghost]

rebased (trivial conflict because some content was added to the sass files in the meantime)

[ghost]

transient error, dealt with in another pull request, this is a known error https://travis-ci.org/freedomofpress/securedrop/builds/312837803?utm_source=github_status&utm_medium=notification

2017-12-07 08:35:56,101 DEBUG DELETE http://127.0.0.1:55140/hub/session/b39ab43b-3f7a-42f0-ad25-9693f5d2df2a {"sessionId": "b39ab43b-3f7a-42f0-ad25-9693f5d2df2a"}
Exception in thread Thread-2950:
Traceback (most recent call last):
  File "/usr/lib/python2.7/threading.py", line 810, in __bootstrap_inner
    self.run()
  File "/usr/lib/python2.7/threading.py", line 763, in run
    self.__target(*self.__args, **self.__kwargs)
  File "/home/travis/build/freedomofpress/securedrop/securedrop/source_app/utils.py", line 81, in async_genkey
    current_app.logger.error(
  File "/usr/local/lib/python2.7/dist-packages/werkzeug/local.py", line 347, in __getattr__
    return getattr(self._get_current_object(), name)
  File "/usr/local/lib/python2.7/dist-packages/werkzeug/local.py", line 306, in _get_current_object
    return self.__local()
  File "/usr/local/lib/python2.7/dist-packages/flask/globals.py", line 51, in _find_app
    raise RuntimeError(_app_ctx_err_msg)
RuntimeError: Working outside of application context.

[toast]

@dachary Yes we could try to convert this into a no JS modal dialog.

Step 1:

Step 2:

Step 3:

In theory it's possible to do this client side with no JS by following a similar pattern to the menu.sass module; using a checkbox to toggle a modal dialog that interrupts the journalist.

But it'll be a bit more complicated because:
When the modal opens, the checkbox's label would need to become an invisible overlay similar to menu.sass that closes the modal when the user clicks outside of it. However, to allow for a CANCEL action and the 'X' with no JS in the window, the close overlay would actually need to cover those two areas of the modal as well (see the blue area in image below). But the DELETE action would need to be placed above the content area to still allow for interaction - this could be done with z-index. Ideally the title and content text would also be placed above the cancel overlay to allow interaction:

Note that the blue area (close overlay) needs to extend to the edges of the screen, I just left it smaller here to see the difference between this and the semi-transparent gray overlay that is behind the entire modal.

All this will also all need to be kept inside the <form> so the post still works when the DELETE inside the modal is clicked (the other DELETE becomes the checkbox label).

I'm not sure if all this will work together neatly in the HTML structure, I haven't made a modal without JS before. Once created though, it could be useful for other dangerous actions elsewhere.

[ghost]

I'm not sure if all this will work together neatly in the HTML structure, I haven't made a modal without JS before.

It would be really nice if it did though. Using the z-index so the action is on top is a clever trick. But, as you point out, I can't imaging how the HTML structure should look like for all this to work together...

[ghost]

@toast I propose we move this discussing in the forum because its scope is larger than this PR. What do you think ?

[toast]

Yep, makes sense @dachary

[ghost]

@toast would like me to implement your proposal ? I'd be happy to draft something for you to review.

P.S. What is your account name is on the forum ? I tried @toast but it did not find you ;-)

[SaptakS]

@toast I agree with your approach. Another approach of creating a modal I may propose is using anchor tags instead of checkbox. We can use the pseudo attribute :target for a div. I have made a small draft example using only css in codepen so that you all can have a look. Maybe this will work.
https://codepen.io/SaptakS/pen/xpByWY
@dachary do have a look.

[ghost]

For the record @SaptakS volunteered to create a new pull request with a draft implementation. Closing this one for now @toast but feel free to re-open when you have time to work on it.