fixed minor issues from PR feedback

Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>
amundsen-io · Aug 11, 2020 · b1c141a · b1c141a
1 parent eed74c7
commit b1c141a
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 36 deletions.
diff --git a/amundsen_application/static/js/components/BrowsePage/constants.ts b/amundsen_application/static/js/components/BrowsePage/constants.ts
@@ -0,0 +1 @@
+export const BROWSE_PAGE_DOCUMENT_TITLE = 'Browse - Amundsen';
diff --git a/amundsen_application/static/js/components/BrowsePage/index.tsx b/amundsen_application/static/js/components/BrowsePage/index.tsx
@@ -7,12 +7,13 @@ import * as DocumentTitle from 'react-document-title';
 import TagsListContainer from 'components/Tags';
 
 import './styles.scss';
+import {BROWSE_PAGE_DOCUMENT_TITLE} from './constants';
 
 export class BrowsePage extends React.Component {
   render() {
     return (
       /* TODO: add expand/collapse behavior */
-      <DocumentTitle title="Browse - Amundsen">
+      <DocumentTitle title={BROWSE_PAGE_DOCUMENT_TITLE}>
         <main className="container">
           <div className="row">
             <div className="col-xs-12">

diff --git a/amundsen_application/static/js/components/HomePage/styles.scss b/amundsen_application/static/js/components/HomePage/styles.scss
@@ -8,10 +8,6 @@
     margin-top: $spacer-4 * 2;
   }
 
-  .title-1 {
-    margin-bottom: $spacer-2;
-  }
-
   .filter-breadcrumb {
     margin-top: 4px;
 

diff --git a/amundsen_application/static/js/components/Tags/TagsList/index.tsx b/amundsen_application/static/js/components/Tags/TagsList/index.tsx
@@ -5,6 +5,7 @@ import * as React from 'react';
 import { Link } from 'react-router-dom';
 
 import ShimmeringTagListLoader from 'components/common/ShimmeringTagListLoader';
+
 import TagInfo from 'components/Tags/TagInfo';
 import { Tag } from 'interfaces';
 
@@ -16,6 +17,7 @@ import {
   BROWSE_TAGS_TITLE,
   BROWSE_PAGE_PATH,
 } from './constants';
+
 import './styles.scss';
 
 export type TagsListProps = StateFromProps & OwnProps;
@@ -28,19 +30,21 @@ export interface StateFromProps {
 }
 
 interface OwnProps {
-  /* determine if we only want curated/popular tags with
-  'Browse more tags' link or the entire tags list */
   shortTagsList?: boolean;
 }
 
 interface TagsListTitleProps {
   titleText: string;
 }
 
+interface TagsListBlockProps {
+  tags: Tag[];
+}
+
 const TagsListTitle: React.FC<TagsListTitleProps> = ({
   titleText,
 }: TagsListTitleProps) => (
-  <h1 className="title-1" id="browse-header">
+  <h1 className="title-1 section-title" id="browse-header">
     {titleText}
   </h1>
 );
@@ -53,10 +57,6 @@ const TagsListLabel: React.FC<TagsListTitleProps> = ({
   </label>
 );
 
-interface TagsListBlockProps {
-  tags: Tag[];
-}
-
 const TagsListBlock: React.FC<TagsListBlockProps> = ({
   tags,
 }: TagsListBlockProps) => {
@@ -73,23 +73,23 @@ const ShortTagsList: React.FC<TagsListProps> = ({
   curatedTags,
   popularTags,
 }: TagsListProps) => {
+  const hasCuratedTags = curatedTags.length > 0;
+  const hasPopularTags = popularTags.length > 0;
   return (
     <div className="short-tag-list">
-      {curatedTags.length === 0 && popularTags.length > 0 && (
+      {!hasCuratedTags && hasPopularTags && (
         <TagsListTitle titleText={POPULAR_TAGS_TITLE} />
       )}
-      {curatedTags.length > 0 && (
+      {hasCuratedTags && (
         <TagsListTitle titleText={CURATED_TAGS_TITLE} />
       )}
-      {curatedTags.length === 0 && popularTags.length > 0 && (
+      {!hasCuratedTags && hasPopularTags && (
         <TagsListBlock tags={popularTags} />
       )}
-      {curatedTags.length > 0 && <TagsListBlock tags={curatedTags} />}
-      <span>
-        <Link to={BROWSE_PAGE_PATH} className="browse-tags-link">
-          {BROWSE_MORE_TAGS_TEXT}
-        </Link>
-      </span>
+      {hasCuratedTags && <TagsListBlock tags={curatedTags} />}
+      <Link to={BROWSE_PAGE_PATH} className="browse-tags-link">
+        {BROWSE_MORE_TAGS_TEXT}
+      </Link>
     </div>
   );
 };
@@ -99,22 +99,25 @@ const LongTagsList: React.FC<TagsListProps> = ({
   popularTags,
   otherTags,
 }: TagsListProps) => {
+  const hasCuratedTags = curatedTags.length > 0;
+  const hasPopularTags = popularTags.length > 0;
+  const hasOtherTags = otherTags.length > 0;
   return (
     <div className="full-tag-list">
-      <TagsListTitle titleText={BROWSE_TAGS_TITLE} />
+      <TagsListTitle titleText={BROWSE_TAGS_TITLE}/>
       <hr className="header-hr" />
-      {curatedTags.length === 0 && popularTags.length > 0 && (
+      {!hasCuratedTags && hasPopularTags && (
         <TagsListLabel titleText={POPULAR_TAGS_TITLE} />
       )}
-      {curatedTags.length > 0 && (
+      {hasCuratedTags && (
         <TagsListLabel titleText={CURATED_TAGS_TITLE} />
       )}
-      {curatedTags.length === 0 && popularTags.length > 0 && (
+      {!hasCuratedTags && hasPopularTags && (
         <TagsListBlock tags={popularTags} />
       )}
-      {curatedTags.length > 0 && <TagsListBlock tags={curatedTags} />}
-      {otherTags.length > 0 && <TagsListLabel titleText={OTHER_TAGS_TITLE} />}
-      {otherTags.length > 0 && <TagsListBlock tags={otherTags} />}
+      {hasCuratedTags && <TagsListBlock tags={curatedTags} />}
+      {hasOtherTags && <TagsListLabel titleText={OTHER_TAGS_TITLE} />}
+      {hasOtherTags && <TagsListBlock tags={otherTags} />}
     </div>
   );
 };

diff --git a/amundsen_application/static/js/components/Tags/TagsList/styles.scss b/amundsen_application/static/js/components/Tags/TagsList/styles.scss
@@ -3,33 +3,37 @@
 
 @import 'variables';
 
-hr.header-hr {
+$browse-tags-link-font-size: 16px;
+
+.header-hr {
   border: 2px solid $brand-color-4;
 }
 
+.section-title {
+  margin-bottom: $spacer-2;
+}
+
 .tags-list {
   margin: 0 -4px;
 }
 
-@import 'variables';
-
 .browse-tags-link {
-  font-size: 16px;
+  font-size: $browse-tags-link-font-size;
   text-decoration: none;
   display: block;
   margin-top: $spacer-1;
 
-  &:hover,
+  &:link,
   &:visited,
-  &:active,
-  &:link {
+  &:hover,
+  &:active {
     text-decoration: none;
   }
 }
 
 .section-label {
   display: block;
-  font-weight: 400;
+  font-weight: $font-weight-body-regular;
   margin-top: $spacer-3;
   margin-bottom: $spacer-2;
 }
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		export const BROWSE_PAGE_DOCUMENT_TITLE = 'Browse - Amundsen';