Merge pull request #249 from edx/mhughes/assets-page-paragon-search

Update Search and Pagination on Assets Page & Edit Image Modal
openedx · Sep 28, 2018 · 89b711e · 89b711e
2 parents 2baa580 + 527bb5a
commit 89b711e
Show file tree

Hide file tree

Showing 20 changed files with 562 additions and 1,015 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -15,7 +15,7 @@
   "license": "AGPL-3.0",
   "dependencies": {
     "@edx/edx-bootstrap": "^1.0.0",
-    "@edx/paragon": "3.1.2",
+    "@edx/paragon": "3.4.8",
     "airbnb-prop-types": "^2.10.0",
     "babel-polyfill": "^6.26.0",
     "classnames": "^2.2.5",
@@ -30,7 +30,6 @@
     "react-dropzone": "^4.2.3",
     "react-intl": "^2.4.0",
     "react-intl-translations-manager": "^5.0.1",
-    "react-paginate": "^5.0.0",
     "react-redux": "^5.0.6",
     "react-transition-group": "^2.2.1",
     "redux": "^3.7.2",

diff --git a/src/components/AssetsList/AssetsList.scss b/src/components/AssetsList/AssetsList.scss
@@ -18,7 +18,6 @@
 
 li span {
   word-wrap: break-word;
-  width: 100%;
 }
 
 // from Bootstrap list-group-item class

diff --git a/src/components/AssetsPage/AssetsPage.test.jsx b/src/components/AssetsPage/AssetsPage.test.jsx
@@ -44,7 +44,7 @@ const renderAssetsTableTest = () => {
 };
 
 const renderPaginationTest = () => {
-  expect(wrapper.find('Connect(Pagination)')).toHaveLength(1);
+  expect(wrapper.find('Connect(InjectIntl(Pagination))')).toHaveLength(1);
 };
 
 const rendersAssetsResultsCountTest = () => {

diff --git a/src/components/AssetsSearch/AssetsSearch.scss b/src/components/AssetsSearch/AssetsSearch.scss
@@ -1,15 +1,9 @@
 @import 'edx-bootstrap';
+@import "~@edx/paragon/src/SearchField/SearchField";
 
-.form-inline {
-  margin-bottom: 0;
-}
 
-.form-inline label {
-  margin-right: 1.0rem;
-  font-weight: 600;
-}
 
-.form-inline button {
-  margin-left: 1.0rem;
-  padding: 0.625rem 0.9rem;
+// Overriding SFE _var-reset rules which interfere with paragon search field functionality
+.search-field .form-group.form-inline {
+  background-color: transparent
 }
diff --git a/src/components/AssetsSearch/AssetsSearch.test.jsx b/src/components/AssetsSearch/AssetsSearch.test.jsx
@@ -1,4 +1,5 @@
 import React from 'react';
+import { SearchField } from '@edx/paragon';
 
 import AssetsSearch from './index';
 import { searchInitial } from './../../data/reducers/assets';
@@ -10,58 +11,54 @@ const defaultProps = {
   courseDetails: {},
 };
 
-let wrapper;
 
 describe('<AssetsSearch />', () => {
   describe('renders', () => {
+    let wrapper;
+    let searchField;
     beforeEach(() => {
       wrapper = mountWithIntl(
         <AssetsSearch
           {...defaultProps}
         />,
       );
+      searchField = wrapper.find(SearchField);
     });
-    it('has label, input, and button in a form element', () => {
-      const searchForm = wrapper.find('form');
-
-      expect(searchForm).toHaveLength(1);
-      expect(searchForm.find('label')).toHaveLength(1);
-      expect(searchForm.find('input[type="search"]')).toHaveLength(1);
-      expect(searchForm.find('button[type="submit"]')).toHaveLength(1);
-    });
-    it('has correct styling', () => {
-      const button = wrapper.find('form').find('button[type="submit"]');
-      expect(wrapper.find('form').hasClass('form-group')).toEqual(true);
-      expect(wrapper.find('form').hasClass('form-inline')).toEqual(true);
-      expect(button.find('span.fa')).toHaveLength(1);
-      expect(button.find('span.fa').hasClass('fa-search')).toEqual(true);
+    it('has a paragon SearchField', () => {
+      expect(searchField).toHaveLength(1);
     });
     it('handles onChange callback correctly', () => {
-      const searchInput = wrapper.find('form input[type="search"]');
-
-      searchInput.simulate('change', { target: { value: 'edX' } });
+      searchField.prop('onChange')('edX');
       expect(wrapper.state('value')).toEqual('edX');
     });
-    it('calls updateSearch when submit button is clicked', () => {
-      const searchSpy = jest.fn();
+    describe('how updateSearch is called', () => {
+      let searchSpy;
 
-      wrapper.setProps({
-        updateSearch: searchSpy,
-      });
+      beforeEach(() => {
+        searchSpy = jest.fn();
 
-      const submitButton = wrapper.find('form button[type="submit"]');
-      const searchInput = wrapper.find('form input[type="search"]');
-      searchInput.simulate('change', { target: { value: 'edX' } });
-      submitButton.simulate('submit');
+        wrapper.setProps({
+          updateSearch: searchSpy,
+        });
+        searchField.prop('onChange')('edX');
+        searchField.prop('onSubmit')();
+      });
 
-      expect(searchSpy).toHaveBeenCalledTimes(1);
-      expect(searchSpy).toHaveBeenCalledWith('edX', defaultProps.courseDetails);
+      it('calls updateSearch when submit button is clicked', () => {
+        expect(searchSpy).toHaveBeenCalledTimes(1);
+        expect(searchSpy).toHaveBeenCalledWith('edX', defaultProps.courseDetails);
+      });
+      it('calls updateSearch when clear button is clicked', () => {
+        searchField.prop('onClear')();
+        expect(searchSpy).toHaveBeenCalledTimes(2);
+        expect(searchSpy).toHaveBeenLastCalledWith('', defaultProps.courseDetails);
+      });
     });
     it('updates search input text when search redux state is updated', () => {
-      const searchInput = wrapper.find('form input[type="search"]');
       wrapper.setProps({ assetsSearch: { search: 'foobar' } }, () => {
+        searchField = wrapper.find(SearchField);
         expect(wrapper.state('value')).toEqual('foobar');
-        expect(searchInput.instance().value).toEqual('foobar');
+        expect(searchField.prop('value')).toEqual('foobar');
       });
     });
   });

diff --git a/src/components/AssetsSearch/displayMessages.jsx b/src/components/AssetsSearch/displayMessages.jsx
@@ -11,6 +11,11 @@ const messages = defineMessages({
     defaultMessage: 'Submit search',
     description: 'Label for search submit button that has a magnifying glass icon',
   },
+  assetsClearSearchButtonLabel: {
+    id: 'assetsClearSearchButtonLabel',
+    defaultMessage: 'Clear search',
+    description: 'Label for a button that clears the search applied to a table.',
+  },
 });
 
 export default messages;
diff --git a/src/components/AssetsSearch/index.jsx b/src/components/AssetsSearch/index.jsx
@@ -1,8 +1,6 @@
 import React from 'react';
-import classNames from 'classnames';
 import PropTypes from 'prop-types';
-import { Button, Icon, InputText } from '@edx/paragon';
-import FontAwesomeStyles from 'font-awesome/css/font-awesome.min.css';
+import { SearchField } from '@edx/paragon';
 
 import WrappedMessage from '../../utils/i18n/formattedMessageWrapper';
 import messages from './displayMessages';
@@ -14,7 +12,6 @@ export default class AssetsSearch extends React.Component {
     this.state = { value: props.assetsSearch.search };
 
     this.handleChange = this.handleChange.bind(this);
-    this.submit = this.submit.bind(this);
   }
 
   componentWillReceiveProps(nextProps) {
@@ -23,49 +20,35 @@ export default class AssetsSearch extends React.Component {
     }
   }
 
-  submit(event) {
+  submit = () => {
     this.props.updateSearch(this.state.value, this.props.courseDetails);
-    event.preventDefault(); // prevent the page from navigating to the form action URL
   }
 
   handleChange(value) {
     this.setState({ value });
   }
 
+  handleClear = () => {
+    // relying on `submit` introduces as state update race
+    // condition
+    this.props.updateSearch('', this.props.courseDetails);
+  }
+
   render() {
-    // TODO: InputText creates its own form-group div. Nesting them is not semantic.
-    // Once Paragon's asInput is refactored, use only one form-group element.
     return (
-      <form
-        className="form-group form-inline justify-content-end"
-        onSubmit={this.submit}
-      >
-        <InputText
-          name="search"
-          className={['form-inline']}
-          type="search"
-          inline
-          label={<WrappedMessage message={messages.assetsSearchInputLabel} />}
-          value={this.state.value}
+      <div className="form-group form-inline justify-content-end">
+        <SearchField
+          onSubmit={this.submit}
           onChange={this.handleChange}
-          inputGroupAppend={(
-            <Button
-              buttonType="primary"
-              type="submit"
-              label={
-                <WrappedMessage message={messages.assetsSearchSubmitLabel}>{
-                  txt => (
-                    <Icon
-                      className={[classNames(FontAwesomeStyles.fa, FontAwesomeStyles['fa-search'])]}
-                      screenReaderText={txt}
-                    />
-                  )
-                }</WrappedMessage>
-              }
-            />
-          )}
+          onClear={this.handleClear}
+          inputLabel={<WrappedMessage message={messages.assetsSearchInputLabel} />}
+          screenReaderText={{
+            clearButton: <WrappedMessage message={messages.assetsClearSearchButtonLabel} />,
+            searchButton: <WrappedMessage message={messages.assetsSearchSubmitLabel} />,
+          }}
+          value={this.state.value}
         />
-      </form>
+      </div>
     );
   }
 }

diff --git a/src/components/CourseChecklist/CourseChecklist.scss b/src/components/CourseChecklist/CourseChecklist.scss
@@ -30,14 +30,6 @@ $success-color: theme-color("success");
   text-decoration: none;
 }
 
-.checklist-item {
-  &:not(:hover) {
-    .checklist-item-link {
-      @include sr_only();
-    }  
-  }
-}
-
 //complete checklist item style
 .checklist-item-complete {
   box-shadow: -5px 0 0 0 $success-color;

diff --git a/src/components/EditImageModal/EditImageModal.test.jsx b/src/components/EditImageModal/EditImageModal.test.jsx
@@ -198,7 +198,7 @@ describe('EditImageModal', () => {
 
 
             it('no Pagination component', () => {
-              expect(getModalBody(editImageModal).find('Connect(Pagination)')).toHaveLength(0);
+              expect(getModalBody(editImageModal).find('Pagination')).toHaveLength(0);
             });
           });
 
@@ -244,7 +244,7 @@ describe('EditImageModal', () => {
             });
 
             it('a Pagination component', () => {
-              expect(getModalBody(editImageModal).find('Connect(Pagination)')).toHaveLength(1);
+              expect(getModalBody(editImageModal).find('Connect(InjectIntl(Pagination))')).toHaveLength(1);
             });
           });
 
@@ -294,7 +294,7 @@ describe('EditImageModal', () => {
             });
 
             it('no Pagination component', () => {
-              expect(getModalBody(editImageModal).find('Connect(Pagination)')).toHaveLength(0);
+              expect(getModalBody(editImageModal).find('Pagination')).toHaveLength(0);
             });
           });
 
@@ -345,7 +345,7 @@ describe('EditImageModal', () => {
             });
 
             it('no Pagination component', () => {
-              expect(getModalBody(editImageModal).find('Connect(Pagination)')).toHaveLength(0);
+              expect(getModalBody(editImageModal).find('Pagination')).toHaveLength(0);
             });
           });
         });

diff --git a/src/components/Pagination/Pagination.scss b/src/components/Pagination/Pagination.scss
@@ -1,4 +1,7 @@
+$pagination-disabled-bg: transparent;
+
 @import 'edx-bootstrap';
+@import '~@edx/paragon/src/Pagination/Pagination';
 
 .pagination {
   justify-content: center;
@@ -11,7 +14,6 @@
 }
 
 .page-item {
-  min-width: 50px;
   text-align: center;
 }
 
@@ -29,3 +31,6 @@
     @extend .disabled;
   }
 }
+.page-item.disabled .page-link {
+  background-color: $pagination-disabled-bg;
+}