From d4ffb6494e10f39207d2d11a969f57ab00d5afe1 Mon Sep 17 00:00:00 2001
From: Arthur Klepchukov <44370126+avkvirtru@users.noreply.github.com>
Date: Sat, 3 Aug 2019 20:28:53 -0400
Subject: [PATCH] UI: clarify revokePolicy feature and filename interface (#94)

* copy: "Start over" => "Reset"

* copy: "Sign out" => "Reset & Sign Out"

* copy: "Revoke File" => "Revoke Policy"

* ui: more consistent modal styles and markup

* ui: move "Reset" to header to appear before signing in

* ui: give "Revoke Policy" the subtle link treatment

move dangerous red treatment into the modal that it triggers

* fix: ellipsize file names that are too long

- show the full name in a tooltip thanks to title attr
- keep .tdf extension when protected

* Update CHANGELOG.md

* fix: test failures

* fix: remove unused imports to appease no-unused-vars in BuildKite
---
 CHANGELOG.md                                  |  9 +-
 src/components/Button/Button.css              | 22 +++++
 src/components/Header/Header.js               |  7 +-
 src/components/Modal/Modal.css                | 29 ++++--
 src/scenes/Document/Document.css              |  5 +-
 .../Document/components/Filename/Filename.css | 12 ++-
 .../Document/components/Filename/Filename.js  | 10 +-
 .../components/RevokeAll/RevokeAll.css        | 96 ++-----------------
 .../components/RevokeAll/RevokeAll.js         | 23 +++--
 .../scenes/DownloadModal/DownloadModal.js     |  6 +-
 src/scenes/Document/scenes/Policy/Policy.css  |  1 -
 src/scenes/Share/Share.js                     |  4 +-
 src/scenes/Share/Share.test.js                |  4 +-
 src/scenes/StayUp/StayUp.css                  | 11 +--
 src/scenes/StayUp/StayUp.js                   |  6 +-
 15 files changed, 103 insertions(+), 142 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ca903c02..d479be6d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,12 +7,19 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## [Unreleased](https://github.com/virtru/protect-and-track/compare/master...HEAD)
 
+- [NO-REF](https://github.com/virtru/protect-and-track/pull/94): _patch_
+  - Clean up "Start over":
+    - copy: "Start over" => "Reset"
+    - move "Reset" (fka "Start over") to header to appear before signing in
+    - copy: "Sign out" => "Reset & Sign Out"
+  - Clarify revoking a policy: - copy: "Revoke File" => "Revoke Policy" - ui: give "Revoke Policy" the subtle link treatment - fix: long file names ellipsize, show in tooltip, and don't conflict with policy revoke
+    fix: ellipsize file names that are too long (but keep .tdf extension and leave room for "Revoke Policy")
 - [DSAT-122](https://github.com/virtru/protect-and-track/pull/85): _patch_
   - Added restricting of downloads when policies have watermark and pfp
 - [NO-REF](https://github.com/virtru/protect-and-track/pull/86): _patch_
   - Clarify default and dragging UI states
 - [DSAT-106](https://github.com/virtru/protect-and-track/pull/78): _patch_
-  - Update UI to better support "Revoke File" workflow
+  - Update UI to better support "Revoke Policy" workflow
 - [NO-REF](https://github.com/virtru/protect-and-track/pull/81): _patch_
   - Fix downloads by not using `FileSaver`
 - [NO-REF](https://github.com/virtru/protect-and-track/pull/80): _patch_
diff --git a/src/components/Button/Button.css b/src/components/Button/Button.css
index d60f06ac..d9fd0b3f 100644
--- a/src/components/Button/Button.css
+++ b/src/components/Button/Button.css
@@ -51,6 +51,28 @@
   border-color: var(--slatePrimary);
 }
 
+.destructiveButton {
+  background: var(--redDarker);
+  border-color: var(--redDarker);
+  color: var(--white);
+}
+.destructiveButton:disabled {
+  background: var(--slateLighter);
+  border-color: var(--slateLighter);
+  box-shadow: none;
+}
+.destructiveButton:hover,
+.destructiveButton:focus {
+  background: var(--slateDarker);
+  border-color: var(--slateDarker);
+  box-shadow: 0 1px 4px 1px var(--slateShadow);
+}
+.destructiveButton:active {
+  background-color: var(--black);
+  border-color: var(--black);
+  color: var(--white);
+}
+
 .alternateButton {
   color: var(--blueDarker);
   background-color: var(--white);
diff --git a/src/components/Header/Header.js b/src/components/Header/Header.js
index a85ac357..50c1f4ed 100644
--- a/src/components/Header/Header.js
+++ b/src/components/Header/Header.js
@@ -18,11 +18,16 @@ const Header = ({ userId }) => {
         <span className="currentUser">
           <span className="userID">{userId}</span>{' '}
           <Button variant="link" onClick={() => Virtru.signOut(userId)} verySmall light>
-            Sign out
+            Reset & Sign Out
           </Button>
         </span>
       );
     }
+    return (
+      <Button variant="link" onClick={() => Virtru.signOut()} verySmall light>
+        Reset
+      </Button>
+    );
   }
   return (
     <div className="headerWrapper">
diff --git a/src/components/Modal/Modal.css b/src/components/Modal/Modal.css
index 2e4ea84d..46a7249d 100644
--- a/src/components/Modal/Modal.css
+++ b/src/components/Modal/Modal.css
@@ -10,15 +10,25 @@
 
 .Modal-content {
   background: white;
-  border: 1px solid --var(white);
-  border-radius: 8px;
+  border: 1px solid var(--white);
+  border-radius: 9px;
   padding: 24px;
-  position: absolute;
-  top: 50%;
-  left: 50%;
-  transform: translate(-50%, -50%);
+  font: normal normal 400 var(--fSizeSmall) / var(--lhSmall) var(--fontPrimary);
+}
+.Modal-content h1 {
+  background-color: transparent;
+  color: var(--slateDarkest);
+  font: 700 var(--fSize-heading-s) / var(--lh-heading-s) var(--fontSecondary);
+  margin-bottom: calc(var(--lh-heading-s) / 2);
+  margin-top: 0;
+  text-align: left;
+}
+.Modal-content button {
+  margin-left: 0;
+  margin-right: 0;
 }
 
+.Modal-content,
 .Modal-content-raw {
   position: absolute;
   top: 50%;
@@ -28,9 +38,12 @@
 
 .Modal-close {
   position: absolute;
-  right: 25px;
-  top: 15px;
+  right: 0;
+  top: 0;
   border: none;
+  border-radius: 9px;
+  padding-right: 24px;
+  padding-top: 24px;
   font-size: 1.5rem;
   cursor: pointer;
 }
diff --git a/src/scenes/Document/Document.css b/src/scenes/Document/Document.css
index 2d033d90..d29d75ed 100644
--- a/src/scenes/Document/Document.css
+++ b/src/scenes/Document/Document.css
@@ -3,10 +3,7 @@
   flex-direction: column;
   width: 100%;
   height: 100%;
-}
-
-.DocumentDetails > .Policy {
-  margin-left: 48px;
+  padding: 24px;
 }
 
 .DocumentWrapper {
diff --git a/src/scenes/Document/components/Filename/Filename.css b/src/scenes/Document/components/Filename/Filename.css
index 07749e31..71c373e9 100644
--- a/src/scenes/Document/components/Filename/Filename.css
+++ b/src/scenes/Document/components/Filename/Filename.css
@@ -4,8 +4,18 @@
   font-size: 25px;
   line-height: 30px;
   font-weight: lighter;
-  padding-left: 48px;
   text-align: left;
+  display: flex;
+  margin-bottom: 12px;
+  margin-top: 0;
+}
+
+.Filename-original {
+  text-overflow: ellipsis;
+  max-width: 370px;
+  white-space: nowrap;
+  overflow: hidden;
+  display: inline-block;
 }
 
 .Filename-tdf {
diff --git a/src/scenes/Document/components/Filename/Filename.js b/src/scenes/Document/components/Filename/Filename.js
index 97c718cd..e7a6f57d 100644
--- a/src/scenes/Document/components/Filename/Filename.js
+++ b/src/scenes/Document/components/Filename/Filename.js
@@ -1,19 +1,15 @@
 import React from 'react';
 
 import RevokeAll from '../RevokeAll/RevokeAll';
-
-import Button from 'components/Button/Button';
-import Virtru from 'utils/VirtruWrapper';
 import './Filename.css';
 
 function Filename({ userId, file, isTdf, isPolicyRevoked, revokePolicy }) {
   return (
     <h2 className="Filename">
-      {file.file.name}
+      <span className="Filename-original" title={file.file.name}>
+        {file.file.name}
+      </span>
       {isTdf && <span className="Filename-tdf">.tdf</span>}
-      <Button variant="link" onClick={() => Virtru.signOut(userId)} small>
-        Start over
-      </Button>
       {isTdf && <RevokeAll isPolicyRevoked={isPolicyRevoked} revokePolicy={revokePolicy} />}
     </h2>
   );
diff --git a/src/scenes/Document/components/RevokeAll/RevokeAll.css b/src/scenes/Document/components/RevokeAll/RevokeAll.css
index a6f3b8bd..f37f8995 100644
--- a/src/scenes/Document/components/RevokeAll/RevokeAll.css
+++ b/src/scenes/Document/components/RevokeAll/RevokeAll.css
@@ -1,93 +1,9 @@
-.RevokeAll {
-  font: var(--fSizeSmall) / var(--lhSmall) var(--fontPrimary);
-  min-height: 36px;
-  margin: 4px;
-  min-width: 94px;
+.button.revokePolicy {
+  /* push this to the end of the flexbox */
+  /* https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Flexible_Box_Layout/Aligning_Items_in_a_Flex_Container#Using_auto_margins_for_main_axis_alignment */
+  margin-left: auto;
 }
 
-.RevokeAll {
-  background: var(--redDarker);
-  border: 1px solid var(--redDarker);
-  border-radius: 3px;
-  color: #fff;
-  font-family: var(--fontPrimary);
-  font-size: 13px;
-  font-weight: bold;
-  line-height: 18px;
-  text-align: center;
-  cursor: pointer;
-}
-.RevokeAll:hover,
-.RevokeAll:focus {
-  background-color: var(--slateDarker);
-  border-color: var(--slateDarker);
-  box-shadow: 0 1px 4px 1px var(--slateShadow);
-  outline: none;
-}
-.RevokeAll:active {
-  background-color: var(--black);
-  border-color: var(--black);
-  box-shadow: 0 1px 4px 1px var(--slateShadow);
-}
-.RevokeAll:disabled {
-  box-shadow: none;
-  background-color: var(--slateLighter);
-  border-color: var(--slateLighter);
-  color: var(--white);
-  cursor: default;
-}
-
-.RevokeAll-container {
-  display: flex;
-  flex-direction: column;
-  width: 300px;
-}
-.RevokeAll-title {
-  color: #2d323b;
-  font-family: var(--fontSecondary);
-  font-size: 16px;
-  line-height: 24px;
-  text-align: left;
-  font-weight: bold;
-}
-.RevokeAll-text {
-  color: #2d323b;
-  font-size: 15px;
-  line-height: 18px;
-  text-align: left;
-  margin: 15px 0 20px;
-  font-family: var(--fontSecondary);
-}
-
-.RevokeAll-confirm {
-  font: var(--fSizeSmall) / var(--lhSmall) var(--fontPrimary);
-  min-height: 36px;
-  margin: 4px;
-  min-width: 94px;
-  border: 1px solid var(--slateDarker);
-  border-radius: 3px;
-  background-color: #fff;
-  cursor: pointer;
-  color: var(--redDarker);
-  font-weight: bold;
-}
-.RevokeAll-confirm:hover,
-.RevokeAll-confirm:focus {
-  box-shadow: 0 1px 4px 1px var(--slateShadow);
-  border-color: var(--redDarker);
-  color: var(--redDarker);
-  outline: none;
-}
-.RevokeAll-confirm:active {
-  box-shadow: 0 1px 4px 1px var(--slateShadow);
-  color: var(--white);
-  border-color: var(--slatePrimary);
-  background-color: var(--redDarker);
-}
-.RevokeAll-confirm:disabled {
-  box-shadow: none;
-  background-color: var(--slateLighter);
-  border-color: var(--slateLighter);
-  color: var(--white);
-  cursor: default;
+.button.destructiveButton {
+  width: 100%;
 }
diff --git a/src/scenes/Document/components/RevokeAll/RevokeAll.js b/src/scenes/Document/components/RevokeAll/RevokeAll.js
index 9be454e9..643cc4a6 100644
--- a/src/scenes/Document/components/RevokeAll/RevokeAll.js
+++ b/src/scenes/Document/components/RevokeAll/RevokeAll.js
@@ -20,26 +20,25 @@ function RevokeAll({ isPolicyRevoked, revokePolicy }) {
   return (
     <>
       <Button
-        className="RevokeAll"
+        small
+        variant="link revokePolicy"
         disabled={isPolicyRevoked}
         onClick={() => {
           setShowWarning(true);
         }}
       >
-        {isPolicyRevoked ? 'Revoked' : 'Revoke File'}
+        {isPolicyRevoked ? 'Revoked' : 'Revoke Policy'}
       </Button>
       {isShowWarning && (
         <Modal onClose={doClose}>
-          <div className="RevokeAll-container">
-            <div className="RevokeAll-title">Revoke File</div>
-            <div className="RevokeAll-text">
-              <p>If you revoke this file, only you will be able to access it.</p>
-              <p>The SDK does not currently support undoing this action.</p>
-            </div>
-            <Button className="RevokeAll-confirm" onClick={doRevoke}>
-              Yes, Revoke This File
-            </Button>
-          </div>
+          <h1>Revoke Policy</h1>
+          <p>If you revoke this file, only you will be able to access it.</p>
+          <p>
+            <strong>The SDK does not currently support undoing this action.</strong>
+          </p>
+          <Button className="button destructiveButton" onClick={doRevoke}>
+            Yes, Revoke This File
+          </Button>
         </Modal>
       )}
     </>
diff --git a/src/scenes/Document/scenes/DownloadModal/DownloadModal.js b/src/scenes/Document/scenes/DownloadModal/DownloadModal.js
index 05408091..47f9c353 100644
--- a/src/scenes/Document/scenes/DownloadModal/DownloadModal.js
+++ b/src/scenes/Document/scenes/DownloadModal/DownloadModal.js
@@ -16,7 +16,7 @@ export default ({ onClose, encrypted, virtruClient }) => {
     return (
       <div className="download-modal">
         <Modal onClose={onClose}>
-          <h4 className="download-title">Download File</h4>
+          <h1>Download File</h1>
           <h2 className="download-subtitle">
             Download this file and open in{' '}
             <a
@@ -58,7 +58,7 @@ export default ({ onClose, encrypted, virtruClient }) => {
     return (
       <div className="download-modal">
         <Modal onClose={onClose}>
-          <h4 className="download-title">Download File</h4>
+          <h1>Download File</h1>
           <span>Share with others:</span>
           <br />
           <Button fullWidth onClick={() => downloadHtml(encrypted)}>
@@ -88,7 +88,7 @@ export default ({ onClose, encrypted, virtruClient }) => {
     return (
       <div className="loading-modal">
         <Modal>
-          <h4 className="loading-modal-title">Loading Policy</h4>
+          <h1>Loading Policy</h1>
           <span>Loading the policy associated to this file...</span>
           <br />
         </Modal>
diff --git a/src/scenes/Document/scenes/Policy/Policy.css b/src/scenes/Document/scenes/Policy/Policy.css
index 18a11ae6..d6258339 100644
--- a/src/scenes/Document/scenes/Policy/Policy.css
+++ b/src/scenes/Document/scenes/Policy/Policy.css
@@ -5,7 +5,6 @@
   font-family: var(--fontPrimary);
   font-size: 13px;
   line-height: 18px;
-  margin: 0 20px;
 }
 
 .Policy-rule {
diff --git a/src/scenes/Share/Share.js b/src/scenes/Share/Share.js
index 5abdf94d..b01355bc 100644
--- a/src/scenes/Share/Share.js
+++ b/src/scenes/Share/Share.js
@@ -14,7 +14,7 @@ function Ico({ type }) {
 }
 
 function Title({ children }) {
-  return <h3 className="Share-title">{children}</h3>;
+  return <h1>{children}</h1>;
 }
 
 function ShareContainer({ children }) {
@@ -174,7 +174,7 @@ function ShareSelect({ setShare, file, recipients, onClose }) {
   };
   return (
     <ShareContainer>
-      <Title>Share {(file && file.name) || 'protected file'}</Title>
+      <Title>Share protected file</Title>
       <ShareButton type="googledrive" onClick={shareToDrive} init={gsuite.init}>
         Google Drive
       </ShareButton>
diff --git a/src/scenes/Share/Share.test.js b/src/scenes/Share/Share.test.js
index 75c98efa..eada8ab1 100644
--- a/src/scenes/Share/Share.test.js
+++ b/src/scenes/Share/Share.test.js
@@ -16,7 +16,7 @@ describe('Share', () => {
     const { getByText, rerender } = render(
       <Share encrypted={file} recipients={['a', 'b']} setShare={setShare} />,
     );
-    expect(getByText('Share a.tdf')).toBeInTheDocument();
+    expect(getByText('Share protected file')).toBeInTheDocument();
     await wait(() => expect(gsuite.init).toHaveBeenCalled());
 
     gsuite.upload.mockReturnValue({ result: { id: 'fake-id' } });
@@ -57,7 +57,7 @@ describe('Share', () => {
     const { getByText, rerender } = render(
       <Share encrypted={file} recipients={['a', 'b']} setShare={setShare} />,
     );
-    expect(getByText('Share a.tdf')).toBeInTheDocument();
+    expect(getByText('Share protected file')).toBeInTheDocument();
     await wait(() => expect(gsuite.init).toHaveBeenCalled());
 
     gsuite.signIn.mockImplementation(() => {
diff --git a/src/scenes/StayUp/StayUp.css b/src/scenes/StayUp/StayUp.css
index 747578c7..d12f325e 100644
--- a/src/scenes/StayUp/StayUp.css
+++ b/src/scenes/StayUp/StayUp.css
@@ -13,7 +13,7 @@
 }
 
 .StayUp-title {
-  color: #2d323b;
+  color: var(--slateDarkest);
   font-family: var(--fontSecondary);
   font-size: 16px;
   line-height: 24px;
@@ -22,12 +22,9 @@
 }
 
 .StayUp-text {
-  color: #2d323b;
-  font-size: 13px;
-  line-height: 18px;
-  text-align: left;
+  color: var(--slateDarkest);
+  font: var(--fSizeSmall) / var(--lhSmall) var(--fontPrimary);
   margin: 15px 0 20px;
-  font-family: var(--fontSecondary);
 }
 
 .hbspt-form .hs-input,
@@ -105,7 +102,7 @@
   display: block;
   width: 100%;
   height: 48px;
-  color: #2d323b;
+  color: var(--slateDarkest);
   font-family: 'Open Sans';
   font-size: 16px;
   margin-bottom: 3px;
diff --git a/src/scenes/StayUp/StayUp.js b/src/scenes/StayUp/StayUp.js
index 6bbfd475..7565ca3d 100644
--- a/src/scenes/StayUp/StayUp.js
+++ b/src/scenes/StayUp/StayUp.js
@@ -22,10 +22,10 @@ function StayUp({ onClose, userId }) {
   return (
     <Modal onClose={onClose}>
       <div className="StayUp-container" ref={scriptWrapper}>
-        <div className="StayUp-title">Stay Up to Date</div>
-        <div className="StayUp-text">
+        <h1>Stay Up to Date</h1>
+        <p>
           Connect with us to get updates about the latest developer tools and resources from Virtru.
-        </div>
+        </p>
       </div>
     </Modal>
   );