Update design for uneditable descriptions (#449)

* Move EditableSection to common

* Update EditableSection

* Code cleanup

* Fix merge

amundsen_application/static/css/_buttons-default.scss

@@ -87,7 +87,7 @@
     background-color: transparent;
     color: $text-secondary;
     box-shadow: none !important;
-    padding: 0px;
+    padding: 0;
     text-align: left;
 
     &:focus,
@@ -102,6 +102,26 @@
     }
   }
 
+  &.btn-flat-icon-dark {
+    border: none;
+    background-color: transparent;
+    color: $text-secondary;
+    box-shadow: none !important;
+    padding: 0;
+    text-align: left;
+
+    &:focus,
+    &:not(.disabled):hover,
+    &:not([disabled]):hover {
+      background-color: transparent;
+      color: $text-primary;
+
+      .icon {
+        background-color: $text-primary;
+      }
+    }
+  }
+
   &.btn-close {
     height: 18px;
     width: 18px;

amundsen_application/static/js/components/DashboardPage/constants.ts

@@ -5,6 +5,7 @@ export const DASHBOARD_OWNER_SOURCE = "dashboard_page_owner"
 export const NO_OWNER_TEXT = 'No owner';
 
 export const ADD_DESC_TEXT = 'Add Description in';
+export const EDIT_DESC_TEXT = 'Click to edit description in';
 
 export const LAST_RUN_SUCCEEDED = 'succeeded';
 export const LAST_RUN_FAILED = 'failed';

amundsen_application/static/js/components/DashboardPage/index.tsx

@@ -9,6 +9,7 @@ import AvatarLabel from 'components/common/AvatarLabel';
 import Breadcrumb from 'components/common/Breadcrumb';
 import BookmarkIcon from 'components/common/Bookmark/BookmarkIcon';
 import Flag from 'components/common/Flag';
+import EditableSection from 'components/common/EditableSection';
 import LoadingSpinner from 'components/common/LoadingSpinner';
 import TabsComponent from 'components/common/TabsComponent';
 import { getDashboard } from 'ducks/dashboard/reducer';
@@ -23,6 +24,7 @@ import { formatDateTimeShort } from '../../utils/dateUtils';
 import ResourceList from 'components/common/ResourceList';
 import {
   ADD_DESC_TEXT,
+  EDIT_DESC_TEXT,
   DASHBOARD_OWNER_SOURCE,
   DASHBOARD_SOURCE,
   LAST_RUN_SUCCEEDED,
@@ -30,7 +32,6 @@ import {
   TABLES_PER_PAGE
 } from 'components/DashboardPage/constants';
 import TagInput from 'components/Tags/TagInput';
-import { EditableSection } from 'components/TableDetail/EditableSection';
 import { ResourceType } from 'interfaces';
 
 import { getSourceDisplayName, getSourceIconClass } from 'config/config-utils';
@@ -158,7 +159,11 @@ export class DashboardPage extends React.Component<DashboardPageProps, Dashboard
               <a id="dashboard-group-link"
                  onClick={ logClick }
                  href={ dashboard.group_url }
-                 target="_blank">{ dashboard.group_name }</a>
+                 target="_blank"
+                 rel="noopener noreferrer"
+              >
+                 { dashboard.group_name }
+              </a>
             </div>
           </div>
           {/* <div className="header-section header-links">links here</div> */}
@@ -167,28 +172,39 @@ export class DashboardPage extends React.Component<DashboardPageProps, Dashboard
                target="_blank"
                href={ dashboard.url }
                onClick={ logClick }
-               className="btn btn-default btn-lg">Open Dashboard</a>
+               className="btn btn-default btn-lg"
+               rel="noopener noreferrer"
+            >
+              Open Dashboard
+            </a>
           </div>
         </header>
         <article className="column-layout-1">
           <section className="left-panel">
-            <div className="section-title title-3">Description</div>
-            {
-              hasDescription &&
-              <div>
-                { dashboard.description }
-              </div>
-            }
-            {
-              !hasDescription &&
-              <a
-               className="edit-link body-2"
-               target="_blank"
-               href={ dashboard.url }
-              >
-               {`${ADD_DESC_TEXT} ${getSourceDisplayName(dashboard.product, ResourceType.dashboard)}`}
-              </a>
-            }
+            <EditableSection
+              title="Description"
+              readOnly={ true }
+              editUrl={ dashboard.url }
+              editText={`${EDIT_DESC_TEXT} ${getSourceDisplayName(dashboard.product, ResourceType.dashboard)}`}
+            >
+              {
+                hasDescription &&
+                <div>
+                  { dashboard.description }
+                </div>
+              }
+              {
+                !hasDescription &&
+                <a
+                 className="edit-link body-2"
+                 target="_blank"
+                 href={ dashboard.url }
+                 rel="noopener noreferrer"
+                >
+                 {`${ADD_DESC_TEXT} ${getSourceDisplayName(dashboard.product, ResourceType.dashboard)}`}
+                </a>
+              }
+            </EditableSection>
             <section className="column-layout-2">
               <section className="left-panel">
                 <div className="section-title title-3">Owners</div>

amundsen_application/static/js/components/TableDetail/ColumnListItem/index.tsx

@@ -12,7 +12,7 @@ import { logClick } from 'ducks/utilMethods';
 import { RequestMetadataType, TableColumn } from 'interfaces';
 
 import './styles.scss';
-import { EditableSection } from 'components/TableDetail/EditableSection';
+import EditableSection from 'components/common/EditableSection';
 
 interface DispatchFromProps {
   openRequestDescriptionDialog: (requestMetadataType: RequestMetadataType, columnName: string) => OpenRequestAction;

amundsen_application/static/js/components/TableDetail/OwnerEditor/index.tsx

@@ -16,7 +16,7 @@ const DEFAULT_ERROR_TEXT = 'There was a problem with the request, please reload
 
 import { GlobalState } from 'ducks/rootReducer';
 import { updateTableOwner } from 'ducks/tableMetadata/owners/reducer';
-import { EditableSectionChildProps } from 'components/TableDetail/EditableSection';
+import { EditableSectionChildProps } from 'components/common/EditableSection';
 import { logClick } from 'ducks/utilMethods';
 
 export interface DispatchFromProps {

amundsen_application/static/js/components/TableDetail/index.tsx

@@ -30,7 +30,7 @@ import WriterLink from 'components/TableDetail/WriterLink';
 import TagInput from 'components/Tags/TagInput';
 import { ResourceType, TableMetadata} from 'interfaces';
 
-import { EditableSection } from 'components/TableDetail/EditableSection';
+import EditableSection from 'components/common/EditableSection';
 
 import { getSourceIconClass, issueTrackingEnabled, notificationsEnabled } from 'config/config-utils';
 

amundsen_application/static/js/components/Tags/TagInput/index.tsx

@@ -10,7 +10,7 @@ import { getAllTags, updateTags} from 'ducks/tags/reducer';
 import { GetAllTagsRequest, UpdateTagsRequest} from 'ducks/tags/types';
 
 import TagInfo from "../TagInfo";
-import { EditableSectionChildProps } from 'components/TableDetail/EditableSection';
+import { EditableSectionChildProps } from 'components/common/EditableSection';
 import { ResourceType, Tag, UpdateMethod, UpdateTagData } from 'interfaces';
 // TODO: Use css-modules instead of 'import'
 import './styles.scss';

amundsen_application/static/js/components/TableDetail/EditableSection/index.spec.tsx → amundsen_application/static/js/components/common/EditableSection/index.spec.tsx

 import * as React from 'react';
 import { shallow } from 'enzyme';
 
-import { EditableSection, EditableSectionProps } from '.';
+import { OverlayTrigger, Popover } from 'react-bootstrap';
+
+import EditableSection, { EditableSectionProps } from '.';
 import TagInput from 'components/Tags/TagInput';
 import { ResourceType } from 'interfaces/Resources';
 
-
 describe("EditableSection", () => {
   const setup = (propOverrides?: Partial<EditableSectionProps>, children?) => {
     const props = {
@@ -72,13 +73,22 @@ describe("EditableSection", () => {
       expect(wrapper.childAt(1).text()).toBe(child);
     });
 
-    it("renders button when readOnly=false", () => {
-      expect(wrapper.find(".edit-button").length).toEqual(1);
+    it("renders edit button correctly when readOnly=false", () => {
+      expect(wrapper.find(".edit-button").props().onClick).toBe(wrapper.instance().toggleEdit);
     });
 
-    it("renders does not add button when readOnly=true", () => {
-      const { wrapper } = setup({readOnly: true}, <TagInput resourceType={ResourceType.table} uriKey={"key"}/>);
-      expect(wrapper.find(".edit-button").length).toEqual(0);
+    describe("renders edit link correctly when readOnly=true", () => {
+      let props;
+      let wrapper;
+      beforeAll(() => {
+        const setupResult = setup({ readOnly: true, editUrl: 'test', editText: 'hello' }, <div/>);
+        props = setupResult.props;
+        wrapper = setupResult.wrapper;
+      });
+
+      it("link links to editUrl", () => {
+        expect(wrapper.find(".edit-button").props().href).toBe(props.editUrl);
+      });
     });
   });
 });

amundsen_application/static/js/components/TableDetail/EditableSection/index.tsx → amundsen_application/static/js/components/common/EditableSection/index.tsx

 import * as React from 'react';
+import { OverlayTrigger, Popover } from 'react-bootstrap';
 
 import './styles.scss';
 
 export interface EditableSectionProps {
   title: string;
   readOnly?: boolean;
+  /* Should be used when readOnly=true to prompt users with a relevant explanation for the given use case */
+  editText?: string;
+  /* Should be used when readOnly=true to link to the source where users can edit the given metadata */
+  editUrl?: string;
 }
 
 interface EditableSectionState {
@@ -37,8 +42,33 @@ export class EditableSection extends React.Component<EditableSectionProps, Edita
     return str.split(new RegExp('[\\s+_]')).map(x => x.charAt(0).toUpperCase() + x.slice(1).toLowerCase()).join(" ");
   }
 
+  renderButton = (): React.ReactNode => {
+     return (
+       <button className={`btn btn-flat-icon edit-button ${(this.state.isEditing ? 'active': '')}`} onClick={ this.toggleEdit }>
+         <img className={`icon icon-small icon-edit ${(this.state.isEditing ? 'icon-color' : '')}`} />
+       </button>
+     );
+  };
+
+  renderReadOnlyButton = (): React.ReactNode => {
+    const { editText, editUrl } = this.props;
+    const popoverHoverFocus = (<Popover id="popover-trigger-hover-focus">{ editText }</Popover>);
+    return (
+      <OverlayTrigger
+       trigger={["hover", "focus"]}
+       placement="top"
+       overlay={popoverHoverFocus}
+      >
+        <a className="btn btn-flat-icon-dark edit-button" href={ editUrl } target="_blank" rel="noopener noreferrer">
+          <img className="icon icon-small icon-edit" />
+        </a>
+      </OverlayTrigger>
+    );
+  };
+
   render() {
-    const childrenWithProps = React.Children.map(this.props.children, child => {
+    const { title, readOnly = false } = this.props;
+    const childrenWithProps = !readOnly ? React.Children.map(this.props.children, child => {
       if (!React.isValidElement(child)) {
         return child;
       }
@@ -46,21 +76,18 @@ export class EditableSection extends React.Component<EditableSectionProps, Edita
         isEditing: this.state.isEditing,
         setEditMode: this.setEditMode,
       });
-    });
+    }) : this.props.children;
 
     return (
       <section className="editable-section">
         <div className="section-title title-3">
-          { EditableSection.convertText(this.props.title) }
-          {
-            !this.props.readOnly &&
-            <button className={"btn btn-flat-icon edit-button" + (this.state.isEditing? " active": "")} onClick={ this.toggleEdit }>
-              <img className={"icon icon-small icon-edit" + (this.state.isEditing? " icon-color" : "")} />
-            </button>
-          }
+          { EditableSection.convertText(title) }
+          { !readOnly ? this.renderButton() : this.renderReadOnlyButton() }
         </div>
         { childrenWithProps }
       </section>
     );
   }
 }
+
+export default EditableSection;

amundsen_application/static/js/components/TableDetail/EditableSection/styles.scss → amundsen_application/static/js/components/common/EditableSection/styles.scss

@@ -9,7 +9,9 @@
   .edit-button {
     opacity: 0;
     margin-left: 4px;
-
+    img {
+      margin-bottom: auto;
+    }
     &.active {
       opacity: 1;
     }

amundsen_application/static/js/components/common/EditableText/index.tsx

@@ -10,7 +10,7 @@ import {
   REFRESH_MESSAGE,
   UPDATE_BUTTON_TEXT
 } from './constants';
-import { EditableSectionChildProps } from 'components/TableDetail/EditableSection';
+import { EditableSectionChildProps } from 'components/common/EditableSection';
 
 export interface StateFromProps {
   refreshValue?: string;