feat: Show more tags in browse and home (#532)

* renamed section header to Popular Tags
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* trying things out
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* set up logic for different exec paths, still have to test sorting function getPopularTags, commented out all tests for TagsList because rewrite will be necessary
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* functionaltiy for home page with both curated and popular tags functionality implemented on TagsList, still need to do some work to make section not show up when no tags exist
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* completed basic implementation of home page tasks with link to browse page, as well as two section brose page displayz'
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* tested and fixed some edge cases
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* lint
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* sorted tags alphabetically within categories
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* some refactoring, adjusted spacing on homepage, fixed styling, implemented title switching base don curated vs popular tags
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* lint and test fix
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* fixed issue to filter out 0 value tags from other tags, keeping curated tags with 0
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* refactor, still need to fix some errors
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* this works
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* replaced a with Link component
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* resolved lint issues
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* fixed minor issues from PR feedback
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* made tag_name key instead of index
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* browse page test issue
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* trying to use mount, not working
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* still having issues when using mount on TagsList for test
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* here are some working tests for tagsList
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* used map for allChildren
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* added fixture fro huge tags list
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* fixes/cleanup
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* added test for shimmer loading render
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

* lint
Signed-off-by: Allison Suarez Miranda <asuarezmiranda@lyft.com>

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

+export const BROWSE_PAGE_DOCUMENT_TITLE = 'Browse - Amundsen';

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

@@ -4,20 +4,21 @@
 import * as React from 'react';
 import * as DocumentTitle from 'react-document-title';
 
-import TagsList from 'components/common/TagsList';
+import TagsListContainer from 'components/common/Tags';
+
+import { BROWSE_PAGE_DOCUMENT_TITLE } from './constants';
+
+import './styles.scss';
 
 export class BrowsePage extends React.Component {
   render() {
     return (
-      <DocumentTitle title="Browse - Amundsen">
+      /* TODO: add expand/collapse behavior */
+      <DocumentTitle title={BROWSE_PAGE_DOCUMENT_TITLE}>
         <main className="container">
           <div className="row">
             <div className="col-xs-12">
-              <h1 className="h3" id="browse-header">
-                Browse Tags
-              </h1>
-              <hr className="header-hr" />
-              <TagsList />
+              <TagsListContainer shortTagsList={false} />
             </div>
           </div>
         </main>

amundsen_application/static/js/components/common/TagsList/styles.scss → amundsen_application/static/js/components/BrowsePage/styles.scss

@@ -2,11 +2,3 @@
 // SPDX-License-Identifier: Apache-2.0
 
 @import 'variables';
-
-hr.header-hr {
-  border: 2px solid $brand-color-4;
-}
-
-.tags-list {
-  margin: 0 -4px;
-}

amundsen_application/static/js/components/BrowsePage/tests/index.spec.tsx

@@ -6,7 +6,7 @@ import * as DocumentTitle from 'react-document-title';
 
 import { shallow } from 'enzyme';
 
-import TagsList from 'components/common/TagsList';
+import TagsListContainer from 'components/common/Tags';
 import { BrowsePage } from '..';
 
 describe('BrowsePage', () => {
@@ -26,16 +26,14 @@ describe('BrowsePage', () => {
       );
     });
 
-    it('renders correct header', () => {
-      expect(wrapper.find('#browse-header').text()).toEqual('Browse Tags');
-    });
-
     it('renders <hr>', () => {
       expect(wrapper.contains(<hr className="header-hr" />));
     });
 
     it('contains TagsList', () => {
-      expect(wrapper.contains(<TagsList />));
+      const expected = 1;
+      const actual = wrapper.find(TagsListContainer).length;
+      expect(actual).toEqual(expected);
     });
   });
 });

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

@@ -35,7 +35,7 @@ import {
   NO_OWNER_TEXT,
   TABLES_PER_PAGE,
 } from 'components/DashboardPage/constants';
-import TagInput from 'components/Tags/TagInput';
+import TagInput from 'components/common/Tags/TagInput';
 import { ResourceType } from 'interfaces';
 
 import { getSourceDisplayName, getSourceIconClass } from 'config/config-utils';

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

 export const SEARCH_BREADCRUMB_TEXT = 'Advanced Search';
 export const HOMEPAGE_TITLE = 'Amundsen Homepage';
-export const TAGS_TITLE = 'Browse Tags';

amundsen_application/static/js/components/HomePage/index.spec.tsx

@@ -9,7 +9,7 @@ import Breadcrumb from 'components/common/Breadcrumb';
 import MyBookmarks from 'components/common/Bookmark/MyBookmarks';
 import PopularTables from 'components/common/PopularTables';
 import SearchBar from 'components/common/SearchBar';
-import TagsList from 'components/common/TagsList';
+import TagsListContainer from 'components/common/Tags';
 
 import { getMockRouterProps } from 'fixtures/mockRouter';
 import { mapDispatchToProps, HomePage, HomePageProps } from '.';
@@ -48,8 +48,7 @@ describe('HomePage', () => {
     });
 
     it('contains TagsList', () => {
-      expect(wrapper.find('#browse-tags-header').text()).toEqual('Browse Tags');
-      expect(wrapper.contains(<TagsList />));
+      expect(wrapper.contains(<TagsListContainer shortTagsList />));
     });
 
     it('contains MyBookmarks', () => {

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

@@ -15,12 +15,8 @@ import PopularTables from 'components/common/PopularTables';
 import { resetSearchState } from 'ducks/search/reducer';
 import { UpdateSearchStateReset } from 'ducks/search/types';
 import SearchBar from 'components/common/SearchBar';
-import TagsList from 'components/common/TagsList';
-import {
-  SEARCH_BREADCRUMB_TEXT,
-  HOMEPAGE_TITLE,
-  TAGS_TITLE,
-} from './constants';
+import TagsListContainer from 'components/common/Tags';
+import { SEARCH_BREADCRUMB_TEXT, HOMEPAGE_TITLE } from './constants';
 
 export interface DispatchFromProps {
   searchReset: () => UpdateSearchStateReset;
@@ -34,6 +30,9 @@ export class HomePage extends React.Component<HomePageProps> {
   }
 
   render() {
+    /* TODO, just display either popular or curated tags,
+    do we want the title to change based on which
+    implementation is being used? probably not */
     return (
       <main className="container home-page">
         <div className="row">
@@ -48,13 +47,7 @@ export class HomePage extends React.Component<HomePageProps> {
               />
             </div>
             <div className="home-element-container">
-              <h2
-                id="browse-tags-header"
-                className="title-1 browse-tags-header"
-              >
-                {TAGS_TITLE}
-              </h2>
-              <TagsList />
+              <TagsListContainer shortTagsList />
             </div>
             <div className="home-element-container">
               <MyBookmarks />

amundsen_application/static/js/components/HomePage/styles.scss

@@ -5,11 +5,7 @@
 
 .home-page {
   .home-element-container {
-    margin-top: 64px;
-  }
-
-  .browse-tags-header {
-    margin-bottom: 32px;
+    margin-top: $spacer-4 * 2;
   }
 
   .filter-breadcrumb {

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

@@ -24,6 +24,7 @@ import BadgeList from 'components/common/BadgeList';
 import BookmarkIcon from 'components/common/Bookmark/BookmarkIcon';
 import Breadcrumb from 'components/common/Breadcrumb';
 import TabsComponent from 'components/common/TabsComponent';
+import TagInput from 'components/common/Tags/TagInput';
 import EditableText from 'components/common/EditableText';
 import LoadingSpinner from 'components/common/LoadingSpinner';
 import Flag from 'components/common/Flag';
@@ -41,7 +42,7 @@ import TableHeaderBullets from 'components/TableDetail/TableHeaderBullets';
 import TableIssues from 'components/TableDetail/TableIssues';
 import WatermarkLabel from 'components/TableDetail/WatermarkLabel';
 import WriterLink from 'components/TableDetail/WriterLink';
-import TagInput from 'components/Tags/TagInput';
+
 import {
   ProgrammaticDescription,
   ResourceType,

amundsen_application/static/js/components/common/Bookmark/MyBookmarks/styles.scss

@@ -6,10 +6,6 @@
 .bookmark-list {
   margin: 32px 0;
 
-  .title-1 {
-    margin-bottom: 32px;
-  }
-
   .tabs-component {
     .nav.nav-tabs {
       margin: 0;

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

@@ -4,7 +4,7 @@
 import * as React from 'react';
 import { shallow } from 'enzyme';
 
-import TagInput from 'components/Tags/TagInput';
+import TagInput from 'components/common/Tags/TagInput';
 import { ResourceType } from 'interfaces/Resources';
 import EditableSection, { EditableSectionProps } from '.';
 

amundsen_application/static/js/components/common/PopularTables/styles.scss

@@ -6,5 +6,4 @@
 .popular-tables-header {
   display: flex;
   flex-direction: row;
-  margin-bottom: 32px;
 }

amundsen_application/static/js/components/common/Tags/TagsList/constants.ts

+export const POPULAR_TAGS_TITLE = 'Popular Tags';
+export const CURATED_TAGS_TITLE = 'Curated Tags';
+export const OTHER_TAGS_TITLE = 'Other Tags';
+export const BROWSE_TAGS_TITLE = 'Browse Tags';
+export const BROWSE_MORE_TAGS_TEXT = 'Browse all tags';
+export const BROWSE_PAGE_PATH = '/browse';

amundsen_application/static/js/components/common/Tags/TagsList/index.spec.tsx

+// Copyright Contributors to the Amundsen project.
+// SPDX-License-Identifier: Apache-2.0
+import * as React from 'react';
+import { Link, BrowserRouter } from 'react-router-dom';
+
+import { shallow } from 'enzyme';
+
+import { strict } from 'assert';
+import { all } from 'redux-saga/effects';
+import { allTestTags } from 'fixtures/metadata/tagsList';
+import TagsList, { TagsListProps } from '.';
+
+const POPULAR_TAGS_NUMBER = 20;
+
+const popularTags = allTestTags.slice(0, POPULAR_TAGS_NUMBER).sort((a, b) => {
+  if (a.tag_name < b.tag_name) return -1;
+  if (a.tag_name > b.tag_name) return 1;
+  return 0;
+});
+
+const otherTags = allTestTags
+  .slice(POPULAR_TAGS_NUMBER, allTestTags.length)
+  .sort((a, b) => {
+    if (a.tag_name < b.tag_name) return -1;
+    if (a.tag_name > b.tag_name) return 1;
+    return 0;
+  });
+
+const setup = (propOverrides?: Partial<TagsListProps>) => {
+  const props = {
+    curatedTags: [],
+    popularTags: [],
+    otherTags: [],
+    ...propOverrides,
+  };
+  const wrapper = shallow<typeof TagsList>(<TagsList {...props} />).dive();
+  return { props, wrapper };
+};
+
+describe('TagsList', () => {
+  describe('render shimmer loader whe isLoading is true', () => {
+    const { wrapper } = setup({
+      popularTags,
+      otherTags,
+      isLoading: true,
+      shortTagsList: true,
+    });
+
+    it('should render ShimmeringTagListLoader', () => {
+      const expected = 1;
+      const actual = wrapper.find('.shimmer-tag-list-loader').length;
+
+      expect(actual).toEqual(expected);
+    });
+  });
+
+  describe('render shortTagsList with popular tags', () => {
+    const { wrapper } = setup({
+      popularTags,
+      otherTags,
+      isLoading: false,
+      shortTagsList: true,
+    });
+
+    it('should render shortTagsList', () => {
+      const expected = 1;
+      const actual = wrapper.find('.short-tag-list').length;
+
+      expect(actual).toEqual(expected);
+    });
+
+    it('should render TagsListTitle', () => {
+      wrapper.children();
+      const expected = 1;
+      const actual = wrapper.childAt(0).shallow().find('.section-title').length;
+
+      expect(actual).toEqual(expected);
+    });
+
+    it('should render TagsListBlock', () => {
+      const expected = 1;
+      const actual = wrapper.childAt(1).shallow().find('.tags-list').length;
+
+      expect(actual).toEqual(expected);
+    });
+
+    it('should render Browse more tags link', () => {
+      const expected = 1;
+      const actual = wrapper.find('.browse-tags-link').length;
+
+      expect(actual).toEqual(expected);
+    });
+  });
+
+  describe('render longTagsList with popular tags', () => {
+    const { wrapper } = setup({
+      popularTags,
+      otherTags,
+      isLoading: false,
+      shortTagsList: false,
+    });
+
+    const allChildren = wrapper.children().map((child) => child.shallow());
+
+    it('should render longTagsList', () => {
+      const expected = 1;
+      const actual = wrapper.find('.full-tag-list').length;
+
+      expect(actual).toEqual(expected);
+    });
+
+    it('should render TagsListTitle', () => {
+      const expected = 1;
+      let actual = 0;
+
+      allChildren.forEach((comp) => {
+        if (comp.find('#browse-header').exists()) {
+          actual++;
+        }
+      });
+
+      expect(actual).toEqual(expected);
+    });
+
+    it('should render TagsListLabels for both sections', () => {
+      const expected = 2;
+      let actual = 0;
+
+      allChildren.forEach((comp) => {
+        if (comp.find('.section-label').exists()) {
+          actual++;
+        }
+      });
+
+      expect(actual).toEqual(expected);
+    });
+
+    it('should render TagsListBlock for both Popular Tags section and Other Tags section', () => {
+      const expected = 2;
+      let actual = 0;
+
+      allChildren.forEach((comp) => {
+        if (comp.find('.tags-list').exists()) {
+          actual++;
+        }
+      });
+      expect(actual).toEqual(expected);
+    });
+  });
+});

amundsen_application/static/js/components/common/Tags/TagsList/index.tsx

+// Copyright Contributors to the Amundsen project.
+// SPDX-License-Identifier: Apache-2.0
+
+import * as React from 'react';
+import { Link } from 'react-router-dom';
+
+import ShimmeringTagListLoader from 'components/common/ShimmeringTagListLoader';
+
+import TagInfo from 'components/common/Tags/TagInfo';
+import { Tag } from 'interfaces';
+
+import {
+  POPULAR_TAGS_TITLE,
+  CURATED_TAGS_TITLE,
+  BROWSE_MORE_TAGS_TEXT,
+  OTHER_TAGS_TITLE,
+  BROWSE_TAGS_TITLE,
+  BROWSE_PAGE_PATH,
+} from './constants';
+
+import './styles.scss';
+
+export type TagsListProps = StateFromProps & OwnProps;
+
+export interface StateFromProps {
+  curatedTags: Tag[];
+  popularTags: Tag[];
+  otherTags?: Tag[];
+  isLoading?: boolean;
+}
+
+interface OwnProps {
+  shortTagsList?: boolean;
+}
+
+interface TagsListTitleProps {
+  titleText: string;
+}
+
+interface TagsListBlockProps {
+  tags: Tag[];
+}
+
+const TagsListTitle: React.FC<TagsListTitleProps> = ({
+  titleText,
+}: TagsListTitleProps) => (
+  <h1 className="title-1 section-title" id="browse-header">
+    {titleText}
+  </h1>
+);
+
+const TagsListLabel: React.FC<TagsListTitleProps> = ({
+  titleText,
+}: TagsListTitleProps) => (
+  <label className="section-label">
+    <span className="section-title title-2">{titleText}</span>
+  </label>
+);
+
+const TagsListBlock: React.FC<TagsListBlockProps> = ({
+  tags,
+}: TagsListBlockProps) => {
+  return (
+    <div id="tags-list" className="tags-list">
+      {tags.map((tag) => (
+        <TagInfo data={tag} compact={false} key={tag.tag_name} />
+      ))}
+    </div>
+  );
+};
+
+const ShortTagsList: React.FC<TagsListProps> = ({
+  curatedTags,
+  popularTags,
+}: TagsListProps) => {
+  const hasCuratedTags = curatedTags.length > 0;
+  const hasPopularTags = popularTags.length > 0;
+  return (
+    <div className="short-tag-list">
+      {!hasCuratedTags && hasPopularTags && (
+        <TagsListTitle titleText={POPULAR_TAGS_TITLE} />
+      )}
+      {hasCuratedTags && <TagsListTitle titleText={CURATED_TAGS_TITLE} />}
+      {!hasCuratedTags && hasPopularTags && (
+        <TagsListBlock tags={popularTags} />
+      )}
+      {hasCuratedTags && <TagsListBlock tags={curatedTags} />}
+      <Link to={BROWSE_PAGE_PATH} className="browse-tags-link">
+        {BROWSE_MORE_TAGS_TEXT}
+      </Link>
+    </div>
+  );
+};
+
+const LongTagsList: React.FC<TagsListProps> = ({
+  curatedTags,
+  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} />
+      <hr className="header-hr" />
+      {!hasCuratedTags && hasPopularTags && (
+        <TagsListLabel titleText={POPULAR_TAGS_TITLE} />
+      )}
+      {hasCuratedTags && <TagsListLabel titleText={CURATED_TAGS_TITLE} />}
+      {!hasCuratedTags && hasPopularTags && (
+        <TagsListBlock tags={popularTags} />
+      )}
+      {hasCuratedTags && <TagsListBlock tags={curatedTags} />}
+      {hasOtherTags && <TagsListLabel titleText={OTHER_TAGS_TITLE} />}
+      {hasOtherTags && <TagsListBlock tags={otherTags} />}
+    </div>
+  );
+};
+
+const TagsList: React.FC<TagsListProps> = ({
+  curatedTags,
+  popularTags,
+  otherTags,
+  isLoading,
+  shortTagsList,
+}: TagsListProps) => {
+  if (isLoading) {
+    return <ShimmeringTagListLoader />;
+  }
+
+  if (shortTagsList) {
+    return (
+      <ShortTagsList curatedTags={curatedTags} popularTags={popularTags} />
+    );
+  }
+
+  return (
+    <LongTagsList
+      curatedTags={curatedTags}
+      popularTags={popularTags}
+      otherTags={otherTags}
+    />
+  );
+};
+
+export default TagsList;

amundsen_application/static/js/components/common/Tags/TagsList/styles.scss

+// Copyright Contributors to the Amundsen project.
+// SPDX-License-Identifier: Apache-2.0
+
+@import 'variables';
+
+$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;
+}
+
+.browse-tags-link {
+  font-size: $browse-tags-link-font-size;
+  text-decoration: none;
+  display: block;
+  margin-top: $spacer-1;
+
+  &:link,
+  &:visited,
+  &:hover,
+  &:active {
+    text-decoration: none;
+  }
+}
+
+.section-label {
+  display: block;
+  font-weight: $font-weight-body-regular;
+  margin-top: $spacer-3;
+  margin-bottom: $spacer-2;
+}

amundsen_application/static/js/components/common/Tags/constants.ts

+export const POPULAR_TAGS_NUMBER = 20;

amundsen_application/static/js/components/common/Tags/index.spec.tsx

+// Copyright Contributors to the Amundsen project.
+// SPDX-License-Identifier: Apache-2.0
+
+import globalState from 'fixtures/globalState';
+
+import { mapStateToProps, mapDispatchToProps } from '.';
+
+jest.mock('config/config-utils', () => ({
+  showAllTags: jest.fn(),
+  getCuratedTags: () => {
+    return ['curated_tag_1'];
+  },
+}));
+
+describe('TagsListContainer', () => {
+  describe('mapDispatchToProps', () => {
+    let dispatch;
+    let props;
+
+    beforeAll(() => {
+      dispatch = jest.fn(() => Promise.resolve());
+      props = mapDispatchToProps(dispatch);
+    });
+
+    it('requests all the tags', () => {
+      const expected = { type: 'amundsen/allTags/GET_REQUEST' };
+
+      props.getAllTags();
+
+      expect(dispatch.mock.calls[0][0]).toEqual(expected);
+    });
+  });
+
+  describe('mapStateToProps', () => {
+    it('sets isLoading on props', () => {
+      const expected = globalState.tags.allTags.isLoading;
+      const actual = mapStateToProps(globalState).isLoading;
+
+      expect(actual).toEqual(expected);
+    });
+
+    let result;
+    let expectedCuratedTags;
+    let expectedOtherTags;
+    let expectedPopularTags;
+    beforeEach(() => {
+      result = mapStateToProps(globalState);
+      const allTags = globalState.tags.allTags.tags;
+
+      expectedCuratedTags = [allTags[0]];
+      expectedOtherTags = [allTags[2], allTags[1]];
+      expectedPopularTags = [];
+    });
+
+    it('sets curatedTags on the props', () => {
+      expect(result.curatedTags).toEqual(expectedCuratedTags);
+    });
+
+    it('sets otherTags on the props', () => {
+      expect(result.otherTags).toEqual(expectedOtherTags);
+    });
+
+    it('sets popularTags on the props', () => {
+      expect(result.popularTags).toEqual(expectedPopularTags);
+    });
+  });
+});

amundsen_application/static/js/components/common/TagsList/index.tsx → amundsen_application/static/js/components/common/Tags/index.tsx

@@ -5,75 +5,109 @@ import * as React from 'react';
 import { connect } from 'react-redux';
 import { bindActionCreators } from 'redux';
 
-import './styles.scss';
-
-import ShimmeringTagListLoader from 'components/common/ShimmeringTagListLoader';
-
-import TagInfo from 'components/Tags/TagInfo';
 import { Tag } from 'interfaces';
 
 import { GlobalState } from 'ducks/rootReducer';
 import { getAllTags } from 'ducks/tags/reducer';
 import { GetAllTagsRequest } from 'ducks/tags/types';
-import { getCuratedTags, showAllTags } from 'config/config-utils';
+import { getCuratedTags } from 'config/config-utils';
+import TagsList from './TagsList';
+
+import { POPULAR_TAGS_NUMBER } from './constants';
 
 export interface StateFromProps {
   curatedTags: Tag[];
+  popularTags: Tag[];
   otherTags: Tag[];
   isLoading: boolean;
 }
 
+interface OwnProps {
+  shortTagsList: boolean;
+}
+
 export interface DispatchFromProps {
   getAllTags: () => GetAllTagsRequest;
 }
 
-export type TagsListProps = StateFromProps & DispatchFromProps;
+export type TagsListContainerProps = StateFromProps &
+  DispatchFromProps &
+  OwnProps;
 
-export class TagsList extends React.Component<TagsListProps> {
+export class TagsListContainer extends React.Component<TagsListContainerProps> {
   componentDidMount() {
     this.props.getAllTags();
   }
 
-  generateTagInfo(tagArray: Tag[]) {
-    return tagArray.map((tag, index) => (
-      <TagInfo data={tag} compact={false} key={index} />
-    ));
-  }
-
   render() {
-    const { isLoading, curatedTags, otherTags } = this.props;
-
-    if (isLoading) {
-      return <ShimmeringTagListLoader />;
-    }
-
+    const {
+      isLoading,
+      curatedTags,
+      popularTags,
+      otherTags,
+      shortTagsList,
+    } = this.props;
     return (
-      <div id="tags-list" className="tags-list">
-        {this.generateTagInfo(curatedTags)}
-        {showAllTags() && curatedTags.length > 0 && otherTags.length > 0 && (
-          <hr />
-        )}
-        {showAllTags() &&
-          otherTags.length > 0 &&
-          this.generateTagInfo(otherTags)}
-      </div>
+      <span className="tag-list">
+        <TagsList
+          curatedTags={curatedTags}
+          popularTags={popularTags}
+          otherTags={otherTags}
+          isLoading={isLoading}
+          shortTagsList={shortTagsList}
+        />
+      </span>
     );
   }
 }
 
 export const mapStateToProps = (state: GlobalState) => {
   // TODO: These functions are selectors, consider moving them into the ducks
-  const curatedTagsList = getCuratedTags();
   const allTags = state.tags.allTags.tags;
-  const curatedTags = allTags.filter(
-    (tag) => curatedTagsList.indexOf(tag.tag_name) !== -1
-  );
-  const otherTags = allTags.filter(
-    (tag) => curatedTagsList.indexOf(tag.tag_name) === -1
-  );
+
+  const allTagsNoZeros = allTags.filter((tag) => tag.tag_count > 0);
+
+  const curatedTagsList = getCuratedTags();
+
+  let curatedTags = [];
+  let popularTags = [];
+  let otherTags = [];
+
+  if (curatedTagsList.length > 0) {
+    // keeping curated tags with zero usage count
+    curatedTags = allTags.filter(
+      (tag) => curatedTagsList.indexOf(tag.tag_name) !== -1
+    );
+    otherTags = allTagsNoZeros
+      .filter((tag) => curatedTagsList.indexOf(tag.tag_name) === -1)
+      .sort((a, b) => {
+        if (a.tag_name < b.tag_name) return -1;
+        if (a.tag_name > b.tag_name) return 1;
+        return 0;
+      });
+  } else {
+    const tagsByUsage = allTagsNoZeros
+      .sort((a, b) => {
+        return a.tag_count - b.tag_count;
+      })
+      .reverse();
+    popularTags = tagsByUsage.slice(0, POPULAR_TAGS_NUMBER).sort((a, b) => {
+      if (a.tag_name < b.tag_name) return -1;
+      if (a.tag_name > b.tag_name) return 1;
+      return 0;
+    });
+    otherTags = tagsByUsage
+      .slice(POPULAR_TAGS_NUMBER, tagsByUsage.length)
+      .sort((a, b) => {
+        if (a.tag_name < b.tag_name) return -1;
+        if (a.tag_name > b.tag_name) return 1;
+        return 0;
+      });
+  }
 
   return {
     curatedTags,
+    popularTags,
     otherTags,
     isLoading: state.tags.allTags.isLoading,
   };
@@ -83,7 +117,7 @@ export const mapDispatchToProps = (dispatch: any) => {
   return bindActionCreators({ getAllTags }, dispatch);
 };
 
-export default connect<StateFromProps, DispatchFromProps>(
+export default connect<StateFromProps, DispatchFromProps, OwnProps>(
   mapStateToProps,
   mapDispatchToProps
-)(TagsList);
+)(TagsListContainer);

amundsen_application/static/js/components/common/TagsList/index.spec.tsx

-// Copyright Contributors to the Amundsen project.
-// SPDX-License-Identifier: Apache-2.0
-
-import * as React from 'react';
-
-import { shallow } from 'enzyme';
-
-import ShimmeringTagListLoader from 'components/common/ShimmeringTagListLoader';
-
-import globalState from 'fixtures/globalState';
-
-import AppConfig from 'config/config';
-
-import { getCuratedTags, showAllTags } from 'config/config-utils';
-import {
-  TagsList,
-  TagsListProps,
-  mapDispatchToProps,
-  mapStateToProps,
-} from '.';
-
-AppConfig.browse.curatedTags = ['test1'];
-
-jest.mock('config/config-utils', () => ({
-  showAllTags: jest.fn(),
-  getCuratedTags: () => {
-    return ['curated_tag_1'];
-  },
-}));
-
-const setup = (propOverrides?: Partial<TagsListProps>) => {
-  const props: TagsListProps = {
-    curatedTags: [
-      {
-        tag_count: 2,
-        tag_name: 'test1',
-      },
-    ],
-    otherTags: [
-      {
-        tag_count: 1,
-        tag_name: 'test2',
-      },
-    ],
-    isLoading: false,
-    getAllTags: jest.fn(),
-    ...propOverrides,
-  };
-
-  const wrapper = shallow(<TagsList {...props} />);
-  return { props, wrapper };
-};
-
-describe('TagsList', () => {
-  describe('componentDidMount', () => {
-    it('calls props.getAllTags', () => {
-      const { props } = setup();
-
-      expect(props.getAllTags).toHaveBeenCalled();
-    });
-  });
-
-  describe('render', () => {
-    it('renders a shimmering loader if props.isLoading is true', () => {
-      const { wrapper } = setup({ isLoading: true });
-
-      expect(wrapper.find(ShimmeringTagListLoader).exists()).toBe(true);
-    });
-
-    it('renders <hr> if curatedTags.length > 0 & otherTags.length > 0 & showAllTags == true', () => {
-      // @ts-ignore
-      showAllTags.mockImplementation(() => true);
-      const { wrapper } = setup();
-      expect(wrapper.find('hr').exists()).toBe(true);
-    });
-
-    it('does not render <hr> if showAllTags is false', () => {
-      // @ts-ignore
-      showAllTags.mockImplementation(() => false);
-      const { wrapper } = setup();
-      expect(wrapper.find('hr').exists()).toBe(false);
-    });
-
-    it('does not render an <hr> if otherTags is empty', () => {
-      // @ts-ignore
-      showAllTags.mockImplementation(() => true);
-      const { wrapper } = setup();
-
-      expect(wrapper.find('#tags-list').find('hr').exists()).toBe(true);
-    });
-
-    it('calls generateTagInfo with curatedTags', () => {
-      const generateTagInfoSpy = jest.spyOn(
-        TagsList.prototype,
-        'generateTagInfo'
-      );
-      const { props, wrapper } = setup();
-      expect(generateTagInfoSpy).toHaveBeenCalledWith(props.curatedTags);
-    });
-
-    it('call generateTagInfo with otherTags', () => {
-      const generateTagInfoSpy = jest.spyOn(
-        TagsList.prototype,
-        'generateTagInfo'
-      );
-      const { props, wrapper } = setup();
-      expect(generateTagInfoSpy).toHaveBeenCalledWith(props.otherTags);
-    });
-  });
-});
-
-describe('mapDispatchToProps', () => {
-  let dispatch;
-  let result;
-
-  beforeEach(() => {
-    dispatch = jest.fn(() => Promise.resolve());
-    result = mapDispatchToProps(dispatch);
-  });
-
-  it('sets getAllTags on the props', () => {
-    expect(result.getAllTags).toBeInstanceOf(Function);
-  });
-});
-
-describe('mapStateToProps', () => {
-  let result;
-  let expectedCuratedTags;
-  let expectedOtherTags;
-  beforeEach(() => {
-    result = mapStateToProps(globalState);
-    const allTags = globalState.tags.allTags.tags;
-    const curatedTagsList = getCuratedTags();
-    expectedCuratedTags = allTags.filter(
-      (tag) => curatedTagsList.indexOf(tag.tag_name) !== -1
-    );
-    expectedOtherTags = allTags.filter(
-      (tag) => curatedTagsList.indexOf(tag.tag_name) === -1
-    );
-  });
-
-  it('sets curatedTags on the props', () => {
-    expect(result.curatedTags).toEqual(expectedCuratedTags);
-  });
-
-  it('sets otherTags on the props', () => {
-    expect(result.otherTags).toEqual(expectedOtherTags);
-  });
-
-  it('sets isLoading on the props', () => {
-    const expectedResult =
-      globalState.tags.allTags.isLoading ||
-      globalState.tags.resourceTags.isLoading;
-    expect(result.isLoading).toEqual(expectedResult);
-  });
-});

amundsen_application/static/js/fixtures/globalState.ts

@@ -191,6 +191,14 @@ const globalState: GlobalState = {
           tag_name: 'other_tag_1',
           tag_count: 15,
         },
+        {
+          tag_name: 'alphabetical_tag_1',
+          tag_count: 5,
+        },
+        {
+          tag_name: 'zero_count_tag_1',
+          tag_count: 0,
+        },
       ],
     },
     resourceTags: {

amundsen_application/static/js/fixtures/metadata/tagsList.ts

+export const allTestTags = [
+  { tag_name: 'test test', tag_count: 3 },
+  { tag_name: 'tao_test', tag_count: 1 },
+  { tag_name: 'real-time', tag_count: 90 },
+  { tag_name: 'tnt-feature', tag_count: 1 },
+  { tag_name: 'test12', tag_count: 1 },
+  { tag_name: 'xtest12', tag_count: 1 },
+  { tag_name: 'ttannis2', tag_count: 2 },
+  { tag_name: 'example', tag_count: 4 },
+  { tag_name: 'audience-feature', tag_count: 1 },
+  { tag_name: 'jinchangtest', tag_count: 1 },
+  { tag_name: 'dispatch', tag_count: 1 },
+  { tag_name: 'test9', tag_count: 48 },
+  { tag_name: 'test2', tag_count: 2 },
+  { tag_name: 'test5', tag_count: 14 },
+  { tag_name: 'test', tag_count: 3 },
+  { tag_name: 'test4', tag_count: 33 },
+  { tag_name: 'testing', tag_count: 26 },
+  { tag_name: 'support', tag_count: 51 },
+  { tag_name: 'abc', tag_count: 4 },
+  { tag_name: 'eta', tag_count: 27 },
+  { tag_name: 'is testing', tag_count: 2 },
+  { tag_name: 'tamika', tag_count: 6 },
+  { tag_name: 'passenger-feature', tag_count: 1 },
+  { tag_name: 'foobar', tag_count: 11 },
+  { tag_name: 'fraud1', tag_count: 83 },
+  { tag_name: 'new_tag', tag_count: 2 },
+  { tag_name: 'testing2', tag_count: 1 },
+  { tag_name: 'test*', tag_count: 1 },
+  { tag_name: 'test_table', tag_count: 1 },
+  { tag_name: 'invalid', tag_count: 2 },
+  { tag_name: 'newtag2', tag_count: 4 },
+  { tag_name: 'trips', tag_count: 45 },
+  { tag_name: 'newvalue3', tag_count: 1 },
+  { tag_name: 'pax', tag_count: 17 },
+  { tag_name: 'ride', tag_count: 22 },
+  { tag_name: 'fraud', tag_count: 7 },
+  { tag_name: 'lbs', tag_count: 9 },
+  { tag_name: 'communications', tag_count: 1 },
+  { tag_name: 'perf', tag_count: 2 },
+  { tag_name: 'a6n', tag_count: 2 },
+  { tag_name: 'integration_test', tag_count: 1 },
+  { tag_name: 'amundsen', tag_count: 3 },
+  { tag_name: 'test1', tag_count: 1 },
+  { tag_name: 'tbs', tag_count: 13 },
+  { tag_name: 'acquisition', tag_count: 1 },
+  { tag_name: 'newtag', tag_count: 1 },
+  { tag_name: 'new_tag2', tag_count: 1 },
+  { tag_name: 'bar', tag_count: 1 },
+  { tag_name: 'test2 test 2', tag_count: 3 },
+  { tag_name: 'test test2', tag_count: 2 },
+  { tag_name: 'hive', tag_count: 1 },
+  { tag_name: 'client', tag_count: 2 },
+  { tag_name: 'hive1', tag_count: 4 },
+  { tag_name: 'passenger', tag_count: 8 },
+  { tag_name: 'tracking', tag_count: 1 },
+  { tag_name: 'hello test', tag_count: 5 },
+  { tag_name: 'newvalue', tag_count: 86 },
+  { tag_name: 'foo', tag_count: 42 },
+  { tag_name: 'bike', tag_count: 10 },
+  { tag_name: 'engagement', tag_count: 1 },
+  { tag_name: 'driver', tag_count: 2 },
+  { tag_name: 'ml-platform', tag_count: 5 },
+  { tag_name: 'event', tag_count: 1 },
+  { tag_name: 'tracking-event', tag_count: 21 },
+  { tag_name: 'finance', tag_count: 9 },
+  { tag_name: 'newValue2', tag_count: 2 },
+  { tag_name: 'regions', tag_count: 16 },
+  { tag_name: 'coco', tag_count: 15 },
+  { tag_name: 'tagger', tag_count: 1 },
+  { tag_name: 'amundsen-test', tag_count: 3 },
+  { tag_name: 'dp-tools', tag_count: 2 },
+];