Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds unread submission icon and select all unread button to source view #1358

Closed
wants to merge 2 commits into from

Conversation

mdrose
Copy link
Contributor

@mdrose mdrose commented Jul 6, 2016

Resolves #1353. Only adds space for an icon if there's at least one unread submission. If all submissions have been read, then the page appears as it would before this PR. Not sure if it's better this way or if it's better to just have an empty space regardless. I was planning on using an open envelope icon for read submissions, making it more clear what the icon means, but, surprisingly, font awesome doesn't have one.

Planning to make the "select all unread" button as well. Also considering adding javascript that removes the icon when the journalist clicks the download button.

Screenshot:
unread

@mdrose mdrose force-pushed the mark_unread_submissions branch from 12e34df to 914dde0 Compare July 7, 2016 05:36
@mdrose
Copy link
Contributor Author

mdrose commented Jul 7, 2016

Added the select unread button. It adds no new jquery, so it's compatible with the direction of #1298. It does add to the existing jquery function that adds in the buttons, however, but merging that should be pretty trivial.

select_unread

@mdrose mdrose changed the title Adds envelope icon to unread submissions in source view Adds unread submission icon and select all unread button to source view Jul 7, 2016
@conorsch
Copy link
Contributor

conorsch commented Aug 3, 2016

Excellent! Thanks very much for this, @mdrose. The new UI is a big improvement, and implements feature in high demand. We do have a conflict with #1298, however. I'm inclined to merge this usability enhancement, and rebase #1298 on top of it if we decide to go forward with excising jQuery entirely.

I was planning on using an open envelope icon for read submissions, making it more clear what the icon means, but, surprisingly, font awesome doesn't have one.

A surprise indeed. While it's a bit counterintuitive that "read" shows no icon:

securedrop-unread-icons

your approach of omitting an icon seems the best strategy for now. Besides, adding the unread notifications is a big enough win that I don't think we need to stress over a corresponding "read" icon.

@redshiftzero
Copy link
Contributor

This is a really great contribution! I just checked this out and tested the following functionality:

  • Verified that the documents displayed as unread are indeed unread
  • Downloaded a document, verified that the unread icon was no longer displayed
  • Used the “Select unread” button to download only the unread documents

Everything looked good to me!

Going forward, we’ll definitely want to have some tests for this new feature. It would be good to break out the unread_subs = [submission for submission in source.submissions if not submission.downloaded] line into a separate function perhaps get_unread_submissions(source) such that unit tests can be written for that piece. Let me know if you’d be interested in working on that! If not, no worries - we can merge in your contribution and make that minor tweak.

@redshiftzero
Copy link
Contributor

Hey @mdrose I pulled your commits into PR #1443 that supersedes this one with unit testing, so I’m closing this PR in favor of #1443. Thanks for your great work on SecureDrop!

@mdrose
Copy link
Contributor Author

mdrose commented Nov 5, 2016

Sorry I didn't get back to you, @redshiftzero. I hadn't checked github in awhile. Thanks for writing unit tests for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants