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

Design updates for SecureDrop 1.0.0, rev1 #4634

Merged
merged 4 commits into from
Aug 15, 2019
Merged

Conversation

eloquence
Copy link
Member

@eloquence eloquence commented Jul 19, 2019

Updates colors throughout to a new "SecureDrop Blue" and a darker
"Grimace Blue", a new "danger" color, and blue gradients on the
index page; updates the favicon and the default logo.

Resolves #4500
Resolves #4501
Resolves #4502
Resolves #4503

Additional tweaks chiefly agreed upon with Nina Alter (@ninavizz)
during pair development:

Global:

  • Removal of some button icons that did not disambiguate meaning
  • Tweaks to default button letter spacing, font weight
  • Pick headline color that fits with new overall theme
  • Ensure consistent use of headline levels
  • Removed unused colors, styles, icons
  • More careful spacing between icons and text on buttons

Source Interface:

  • New trashcan icon for deleting replies
  • Flipped order of introductory text on submit page by importance

Journalist Interface:

  • Restyled timestamps, "0 docs, 1 message" type counters to be less easily confused with buttons
  • Restyled "Select all" type buttons to not have checkbox icons on them, to ensure they're not confused with actual checkboxes
  • Added spacing between Select buttons and other actions, to make logical distinction more obvious
  • Removed download icon from each table row, as it did not convey sufficient meaning to justify keeping it
  • Removed "Reply" icon from Reply headline, and left-aligned it, for consistency with other headlines
  • Made textbox the same width as the table above it, and removed fixed column width, to ensure proper scaling across viewport sizes
  • Right-aligned "Submit" button consistent with other forms
  • Removed hover from "X" of modals, as it did not really add value here

Admin Interface:

  • Made it more obvious when the Delete icon is disabled

Out of scope for this rev:

  • String changes
  • Typography changes
  • Docs screenshot updates (will be done after final rev)

Test plan

  • Check the changes against the changelog above.
  • Verify all paths through the Source Interface, including:
    • submitting a file or message
    • changing the UI language
    • deleting a reply from a journalist
  • Verify all paths through the Journalist Interface, including:
    • "Flag for reply" flow
    • replying to a source
    • deletion of collections and sources
    • deletion of replies by sources
    • changing codename of a source
    • adding a user
    • other administrative functions

When stepping through these paths, make note of any obvious design regressions, or design changes which you are uncertain about ("is it intentional that the hover behavior between these two buttons is inconsistent", etc.).

Deployment considerations

  • Screenshots in docs will have to be updated after final rev
  • We'll reach out to news organizations prior to 1.0.0 to keep them informed about the logo change, so they can account for it on landing pages and in other materials
  • We'll prominently explain the UI changes in the changelog for 1.0.0

Screenshots

Screen Shot 2019-07-30 at 17 35 48-fullpage
Screen Shot 2019-07-30 at 17 35 50-fullpage
Screen Shot 2019-07-30 at 17 36 13-fullpage
Screen Shot 2019-07-30 at 17 36 26-fullpage
Screen Shot 2019-07-30 at 17 36 33-fullpage
Screen Shot 2019-07-30 at 17 37 06-fullpage
Screen Shot 2019-07-30 at 17 37 14-fullpage
Screen Shot 2019-07-30 at 17 37 49-fullpage
Screen Shot 2019-07-30 at 17 38 06-fullpage

@eloquence eloquence force-pushed the logo-and-color-refresh-2019 branch from 09c5e80 to 1ed3f3c Compare July 19, 2019 23:02
@eloquence
Copy link
Member Author

Asks for @ninavizz:

Huge thanks for getting us this far, really looking forward to doing this refresh as part of 1.0.0 :)

border-width: 1.5px
border-image-source: linear-gradient(145deg, #0096dc, #005db7)
border-image-slice: 1
border: 1px solid
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need to remove border: 1px solid here so it doesn't override the gradient border.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks! (Should be fixed.)

@rmol
Copy link
Contributor

rmol commented Jul 22, 2019

Took a spin through the changes to check the source interface against the four issues listed, and they look pretty good.

Made a comment about the one bug I found, preventing the border image gradient on the source index page.

The only other thing I noticed was that setting the alpha to 20% for the cloud icon on the submit page would make it too light, IMO -- but that item is as yet unchecked on the issue page, so maybe it's not yet nailed down.

@eloquence eloquence force-pushed the logo-and-color-refresh-2019 branch 3 times, most recently from 532737c to a2237e4 Compare July 26, 2019 04:33
@eloquence eloquence force-pushed the logo-and-color-refresh-2019 branch 3 times, most recently from 65e5c93 to 1de835e Compare July 31, 2019 00:59
@eloquence
Copy link
Member Author

@ninavizz and I made a fair number of smaller tweaks as we audited the colors and styles used throughout the Journalist and Source UI in the last few days. I've attempted to provide a complete changelog in the PR and in the commit summary.

To manage scope, I'm going to call this rev1; at this point, updates to this PR will be to identify issues flagged during review. We'll be discussing the scope of a potential rev2 in the next UX meeting, on August 1.

@eloquence eloquence changed the title [WIP] Logo & color refresh for SecureDrop 1.0.0 [WIP] Design updates for SecureDrop 1.0.0, rev1 Jul 31, 2019
@eloquence eloquence force-pushed the logo-and-color-refresh-2019 branch 9 times, most recently from bebec52 to bdbbe8c Compare August 1, 2019 06:09
@eloquence eloquence marked this pull request as ready for review August 1, 2019 07:08
@eloquence eloquence changed the title [WIP] Design updates for SecureDrop 1.0.0, rev1 Design updates for SecureDrop 1.0.0, rev1 Aug 1, 2019
@rmol
Copy link
Contributor

rmol commented Aug 2, 2019

👍 LGTM. The border bug is fixed, and the test changes are good.

@eloquence
Copy link
Member Author

@ninavizz Just a note that the full-size logo in this PR still uses the old keyhole cut; could you provide me with a 500 x 449 resolution transparent PNG version with the revised cut?

@codecov-io
Copy link

Codecov Report

Merging #4634 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #4634   +/-   ##
========================================
  Coverage    82.67%   82.67%           
========================================
  Files           45       45           
  Lines         3122     3122           
  Branches       338      338           
========================================
  Hits          2581     2581           
  Misses         454      454           
  Partials        87       87

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff95b55...cd30c12. Read the comment docs.

@codecov-io
Copy link

codecov-io commented Aug 10, 2019

Codecov Report

Merging #4634 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #4634   +/-   ##
========================================
  Coverage    82.67%   82.67%           
========================================
  Files           45       45           
  Lines         3122     3122           
  Branches       338      338           
========================================
  Hits          2581     2581           
  Misses         454      454           
  Partials        87       87

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update de8cbcf...0cc9bc7. Read the comment docs.

Updates colors throughout to a new "SecureDrop Blue" and a darker
"Grimace Blue", a new "danger" color, and blue gradients on the
index page; updates the favicon and the default logo.

Resolves #4500
Resolves #4501
Resolves #4502
Resolves #4503

Additional tweaks chiefly agreed upon with Nina Alter (@ninavizz)
during pair development:

Global:
- Removal of some button icons that did not disambiguate meaning
- Tweaks to default button letter spacing, font weight
- Pick headline color that fits with new overall theme
- Ensure consistent use of headline levels
- Removed unused colors, styles, icons
- More careful spacing between icons and text on buttons
- Consistent use of upper case on large buttons

Source Interface:
- New trashcan icon for deleting replies
- Flipped order of introductory text on submit page by importance

Journalist Interface:
- Restyled timestamps, "0 docs, 1 message" type counters to be
  less easily confused with buttons
- Restyled "Select all" type buttons to not have checkbox icons
  on them, to ensure they're not confused with actual checkboxes
- Added spacing between Select buttons and other actions, to
  make logical distinction more obvious
- Removed download icon from each table row, as it did not convey
  sufficient meaning to justify keeping it
- Removed "Reply" icon from Reply headline, and left-aligned it,
  for consistency with other headlines
- Made textbox the same width as the table above it, and removed
  fixed column width, to ensure proper scaling across viewport
  sizes
- Right-aligned "Submit" button consistent with other forms
- Removed hover from "X" of modals, as it did not really add
  value here

Admin Interface:
- Made it more obvious when the Delete icon is disabled

Out of scope for this rev:
- String changes
- Typography changes
- Docs screenshot updates (will be done after final rev)
We are no longer changing icon hover state, so it's not useful to
continue to test for it.
@eloquence eloquence force-pushed the logo-and-color-refresh-2019 branch from cd30c12 to 064de4a Compare August 13, 2019 04:47
@eloquence
Copy link
Member Author

(Rebased.)

border-top-width: 0
border-bottom-width: 0

.index-left
background: $color_purple_medium
background-image: linear-gradient(140deg, #0096dc, #005db7)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would using variables for theses colors make sense here, since they are reused later?

Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @eloquence @ninavizz this looks great! I've noticed a couple of (what seem to me) inconsistencies in the colors, specifically warning/banners. What do you think? I'll defer to @ninavizz for the final review, otherwise this looks good to me, based on the testing below.

  1. The Tor Security settings banner remaining the same color and not the same as securedrop.org (should it be urgent coral #f360000 ?:
    tor-banner
    sdo-tor-banner
  2. The yellow message window appears more muted, less fluorescent than the other colors on the page:
    image
  3. Same as the above comment, to a lesser extent with the green message box:
    thank you . Perhaps a color closer to the GitHub green (which is quite close to the exiting) might be better.

Also performed functional testing as follows:

VM testing

Clean install (staging)

  • Navigated through both interfaced per the test plan and did not observe any issues (other than what was described above
  • No AppArmor errors

VM Upgrade scenario (using diff below [0] due to #4659)

  • Navigated through both interfaced per the test plan and did not observe any issues (other than what was described above
  • No AppArmor errors

[0] :

diff --git a/molecule/upgrade/side_effect.yml b/molecule/upgrade/side_effect.yml
index a33bddcd8..250535d33 100644
--- a/molecule/upgrade/side_effect.yml
+++ b/molecule/upgrade/side_effect.yml
@@ -3,10 +3,10 @@
   hosts: securedrop
   become: yes
   tasks:
-    - name: Perform safe upgrade
+    - name: Perform dist-upgrade
       apt:
         update_cache: yes
-        upgrade: yes
+        upgrade: dist
 
 - name: Lay out app testing deps
   hosts: securedrop_application_server

@eloquence
Copy link
Member Author

eloquence commented Aug 13, 2019

Thanks for the comments and the careful review, @emkll! You're right that those colors are still not ideal. We kept them out of scope for now to keep scope constrained, but they're easy to change, and your comment encouraged us to poke a little. Nina and I will sync up on that tomorrow and may make some final tweaks here.

@ninavizz
Copy link
Member

ninavizz commented Aug 14, 2019

Thank you for flagging the design concerns @emkll! I agree with all of the points you've made.

The Get Codename page and all Messaging components need more love than just color tweeks, unfortunately. Adjusting the colors, alone, with the visual language currently used in both would potentially make those parts of the UI look "more broken," than simply untouched by the current round of UI updates. If that makes sense.

I'll defer to @eloquence on how to scope agreed to updates across which PRs; this one (Rev1) and a Rev2 are being created for 1.0.0. Per your mention in your review, however, Erik and I did just agree to extend the scope of the Rev2 items to include Get Codename page updates... and possibly an update to the Codename Widget on the Submit page. Both of which, are certainly needed.

Messaging I'd like to revisit holistically in either a point release, or in 1.1.0. At that time colors, iconography, styling, and text will all be overhauled. The Tor messaging banner(s) on the index page, tho, I do consider to be outside the scope of that, as I consider it to be its own design problem. We also have yet to get clarification on the exact urgency of that ask—which will factor heavily into which direction we take in that design solution. Not sure if you've got that on your own radar atm to look into, or if I should create a new GH Issue for that? Pls advise.

TL;DR, this PR I'm kosher thumbs-up'ing, with the caveat we get the flagged Get Codename updates into the Rev2 PR.

Messaging and Tor things that have been deferred until later, I've documented in this Issue in the UX Repo for consideration +1.0.1

@eloquence
Copy link
Member Author

That makes sense to me @ninavizz! If the changes that are in here are good to go, I'd love to get those into develop so we can all sit with them for a while, and so we have a guaranteed baseline to work with in case other improvements slip. My order of priorities for rev2 is to clean up the index page a bit more per the spec, and then take a stab at the codename UI refresh.

emkll
emkll previously approved these changes Aug 14, 2019
Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ninavizz for the review! Approving based on my previous review and review provided by @ninavizz

@@ -72,7 +72,7 @@ <h2>{{ gettext('Reset Password') }}</h2>
<br>
<span id="password" class="password">{{ password }}</span>
</p>
<button class="sd-button btn" type="submit" id="reset-password"><i class="fas fa-sync"></i> {{ gettext('Reset Password') }}</button>
<button type="submit" id="reset-password"><i class="fas fa-sync"></i> {% filter upper %}{{ gettext('Reset Password') }}{% endfilter %}</button>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this work with all locales/languages? We should explicitly test this as part of QA

@eloquence
Copy link
Member Author

(Hold on merge for one follow-up commit, we agreed to use text-transform: uppercase where appropriate, consistent w/ previous discussion in #1587.)

@emkll emkll dismissed their stale review August 14, 2019 19:48

Dismissing per #4634 (comment) , will re-review

@eloquence
Copy link
Member Author

.upper class for using text-transform:uppercase on strings that would otherwise be touched in this PR introduced in 0cc9bc7.

Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approving once more, css changes in 0cc9bc7 look good to me

@emkll emkll merged commit 9af7898 into develop Aug 15, 2019
@rmol rmol mentioned this pull request Sep 16, 2019
23 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants