From a94b7ca12e6a087670fc48932270ac307987ea4d Mon Sep 17 00:00:00 2001 From: Carl Kibler Date: Tue, 17 Sep 2019 10:22:36 -0600 Subject: [PATCH] update to newer InstUI Select component closes ADMIN-2774 flag=none test plan: - In a Blueprint course, go to Associations in the sidebar - Verify term and subaccount select boxes work well and the course list updates after selection Change-Id: Ib55e8a01653fa8cd13aa7779ecf27bcd05861108 Reviewed-on: https://gerrit.instructure.com/212047 Tested-by: Jenkins Reviewed-by: Jon Willesen QA-Review: Jon Willesen Product-Review: Carl Kibler --- .../components/CourseFilter.js | 56 ++--- .../components/CoursePicker.js | 2 +- .../components/CourseSidebar.js | 2 +- .../components/CourseFilterSpec.js | 211 +++++++++++------- spec/selenium/helpers/blueprint_common.rb | 10 +- .../blueprint_associations_spec.rb | 6 +- .../master_courses/course_picker_spec.rb | 8 +- 7 files changed, 181 insertions(+), 114 deletions(-) diff --git a/app/jsx/blueprint_courses/components/CourseFilter.js b/app/jsx/blueprint_courses/components/CourseFilter.js index f3e98263163..05e0eb9a7d0 100644 --- a/app/jsx/blueprint_courses/components/CourseFilter.js +++ b/app/jsx/blueprint_courses/components/CourseFilter.js @@ -20,9 +20,9 @@ import I18n from 'i18n!blueprint_settingsCourseFilter' import React from 'react' import PropTypes from 'prop-types' import {TextInput} from '@instructure/ui-forms' -import Select from '@instructure/ui-core/lib/components/Select' import {ScreenReaderContent} from '@instructure/ui-a11y' import {Grid} from '@instructure/ui-layout' +import CanvasSelect from 'jsx/shared/components/CanvasSelect' import propTypes from '../propTypes' const { func } = PropTypes @@ -61,9 +61,7 @@ export default class CourseFilter extends React.Component { onChange = () => { this.setState({ - search: this.getSearchText(), - term: this.termInput.value, - subAccount: this.subAccountInput.value, + search: this.getSearchText() }) } @@ -87,9 +85,7 @@ export default class CourseFilter extends React.Component { setTimeout(() => { if (this.state.isActive) { const search = this.searchInput.value - const term = this.termInput.value - const subAccount = this.subAccountInput.value - const isEmpty = !search && !term && !subAccount + const isEmpty = !search if (isEmpty && !this.wrapper.contains(document.activeElement)) { this.setState({ @@ -101,6 +97,20 @@ export default class CourseFilter extends React.Component { } render () { + const termOptions = [ + {I18n.t('Any Term')}, + ...this.props.terms.map(term => ( + {term.name} + )) + ] + + const subAccountOptions = [ + {I18n.t('Any Sub-Account')}, + ...this.props.subAccounts.map(account => ( + {account.name} + )) + ] + return (
{ this.wrapper = c }}> @@ -119,38 +129,30 @@ export default class CourseFilter extends React.Component { /> - + {termOptions} + - + {subAccountOptions} + diff --git a/app/jsx/blueprint_courses/components/CoursePicker.js b/app/jsx/blueprint_courses/components/CoursePicker.js index 70762edbdda..9b461ed22ad 100644 --- a/app/jsx/blueprint_courses/components/CoursePicker.js +++ b/app/jsx/blueprint_courses/components/CoursePicker.js @@ -101,7 +101,7 @@ export default class CoursePicker extends React.Component { }) } - // when user clicks "Courses" button to toggle visibliity + // when user clicks "Courses" button to toggle visibility onToggleCoursePicker = (event, isExpanded) => { this.setState({isExpanded}) } diff --git a/app/jsx/blueprint_courses/components/CourseSidebar.js b/app/jsx/blueprint_courses/components/CourseSidebar.js index 7644c4aa34d..fa08d01bee9 100644 --- a/app/jsx/blueprint_courses/components/CourseSidebar.js +++ b/app/jsx/blueprint_courses/components/CourseSidebar.js @@ -113,7 +113,7 @@ export default class CourseSidebar extends Component { }), }, children: ( - {I18n.t('Loading assotiations...')}
}> + {I18n.t('Loading associations...')}}> ), diff --git a/spec/javascripts/jsx/blueprint_courses/components/CourseFilterSpec.js b/spec/javascripts/jsx/blueprint_courses/components/CourseFilterSpec.js index e022ffe1e50..f17d0be5335 100644 --- a/spec/javascripts/jsx/blueprint_courses/components/CourseFilterSpec.js +++ b/spec/javascripts/jsx/blueprint_courses/components/CourseFilterSpec.js @@ -17,95 +17,152 @@ */ import React from 'react' +import {render} from '@testing-library/react' import * as enzyme from 'enzyme' import CourseFilter from 'jsx/blueprint_courses/components/CourseFilter' import getSampleData from '../getSampleData' -QUnit.module('CourseFilter component') - const defaultProps = () => ({ subAccounts: getSampleData().subAccounts, - terms: getSampleData().terms, + terms: getSampleData().terms }) +let fixtures -test('renders the CourseFilter component', () => { - const tree = enzyme.shallow() - const node = tree.find('.bca-course-filter') - ok(node.exists()) -}) +QUnit.module('CourseFilter', hooks => { + hooks.beforeEach(() => { + fixtures = document.getElementById('fixtures') + fixtures.innerHTML = '' + }) -test('onChange fires with search filter when text is entered in search box', (assert) => { - const done = assert.async() - const props = defaultProps() - props.onChange = (filter) => { - equal(filter.search, 'giraffe') - done() - } - const tree = enzyme.mount() - const input = tree.find('TextInput input') - input.instance().value = 'giraffe' - input.simulate('change') -}) + hooks.afterEach(() => { + fixtures.innerHTML = "" + }) -test('onChange fires with term filter when term is selected', (assert) => { - const done = assert.async() - const props = defaultProps() - props.onChange = (filter) => { - equal(filter.term, '1') - done() - } - const tree = enzyme.mount() - const input = tree.find('select').at(0) - input.instance().value = '1' - input.simulate('change') -}) + test('renders the CourseFilter component', () => { + const tree = enzyme.shallow() + const node = tree.find('.bca-course-filter') + ok(node.exists()) + }) -test('onChange fires with subaccount filter when a subaccount is selected', (assert) => { - const done = assert.async() - const props = defaultProps() - props.onChange = (filter) => { - equal(filter.subAccount, '1') - done() - } - const tree = enzyme.mount() - const input = tree.find('select').at(1) - input.instance().value = '1' - input.simulate('change') -}) + test('onChange fires with search filter when text is entered in search box', (assert) => { + const done = assert.async() + const props = defaultProps() + props.onChange = (filter) => { + equal(filter.search, 'giraffe') + done() + } + const tree = enzyme.mount() + const input = tree.find('input[type="search"]') + input.instance().value = 'giraffe' + input.simulate('change') + }) -test('onActivate fires when filters are focussed', () => { - const props = defaultProps() - props.onActivate = sinon.spy() - const tree = enzyme.mount() - const input = tree.find('TextInput input') - input.simulate('focus') - ok(props.onActivate.calledOnce) -}) + test('onActivate fires when filters are focused', () => { + const props = defaultProps() + props.onActivate = sinon.spy() + const tree = enzyme.mount() + const input = tree.find('input[type="search"]') + input.simulate('focus') + ok(props.onActivate.calledOnce) + }) + + test('onChange not fired when < 3 chars are entered in search text input', (assert) => { + const done = assert.async() + const props = defaultProps() + props.onChange = sinon.spy() + const tree = enzyme.mount() + const input = tree.find('input[type="search"]') + input.instance().value = 'aa' + input.simulate('change') + setTimeout(() => { + equal(props.onChange.callCount, 0) + done() + }, 0) + }) + + test('onChange fired when 3 chars are entered in search text input', (assert) => { + const done = assert.async() + const props = defaultProps() + props.onChange = sinon.spy() + const tree = enzyme.mount() + const input = tree.find('input[type="search"]') + input.instance().value = 'aaa' + input.simulate('change') + setTimeout(() => { + ok(props.onChange.calledOnce) + done() + }, 0) + }) + + QUnit.module('CourseFilter > Filter behavior', suiteHooks => { + let container + let component + let select + + suiteHooks.afterEach(() => { + component.unmount() + container.remove() + }) + + function renderComponent(props) { + return render(, {container}) + } + + function clickToExpand() { + select.click() + } + + function getOptionsList() { + const optionsListId = select.getAttribute('aria-controls') + return document.getElementById(optionsListId) + } + + function getOption(optionLabel) { + return getOptions().find($option => $option.textContent.trim() === optionLabel) + } + + function getOptions() { + return [...getOptionsList().querySelectorAll('[role="option"]')] + } + + function getOptionLabels() { + return getOptions().map(option => option.textContent.trim()) + } + + function selectOption(optionLabel) { + getOption(optionLabel).click() + } + + test('onChange fires with term filter when term is selected', (assert) => { + const done = assert.async() + const props = defaultProps() + props.onChange = (filter) => { + equal(filter.term, '1') + done() + } + container = document.body.appendChild(document.createElement('div')) + component = renderComponent(props) + select = container.querySelectorAll('input[type="text"]')[0] + + clickToExpand() + selectOption('Term One') + }) + + test('onChange fires with subaccount filter when a subaccount is selected', (assert) => { + const done = assert.async() + const props = defaultProps() + props.onChange = (filter) => { + equal(filter.subAccount, '2') + done() + } + container = document.body.appendChild(document.createElement('div')) + component = renderComponent(props) + select = container.querySelectorAll('input[type="text"]')[1] + + clickToExpand() + selectOption('Account Two') + }) + }) -test('onChange not fired when < 3 chars are entered in search text input', (assert) => { - const done = assert.async() - const props = defaultProps() - props.onChange = sinon.spy() - const tree = enzyme.mount() - const input = tree.find('input[type="search"]') - input.instance().value = 'aa' - input.simulate('change') - setTimeout(() => { - equal(props.onChange.callCount, 0) - done() - }, 0) -}) -test('onChange fired when 3 chars are entered in search text input', (assert) => { - const done = assert.async() - const props = defaultProps() - props.onChange = sinon.spy() - const tree = enzyme.mount() - const input = tree.find('input[type="search"]') - input.instance().value = 'aaa' - input.simulate('change') - setTimeout(() => { - ok(props.onChange.calledOnce) - done() - }, 0) }) diff --git a/spec/selenium/helpers/blueprint_common.rb b/spec/selenium/helpers/blueprint_common.rb index 4d5752f1012..802a390cb6f 100644 --- a/spec/selenium/helpers/blueprint_common.rb +++ b/spec/selenium/helpers/blueprint_common.rb @@ -149,7 +149,15 @@ module BlueprintCourseCommon expect(details_wrapper).to contain_css('.bca-table__course-row') end - # reutrn the holding the list of avaiable courses + def term_options + INSTUI_Select_options('#termsFilter').map(&:text) + end + + def sub_account_options + INSTUI_Select_options('#subAccountsFilter').map(&:text) + end + + # return the holding the list of available courses def available_courses_table f('.bca-table__content-wrapper tbody') end diff --git a/spec/selenium/master_courses/blueprint_associations_spec.rb b/spec/selenium/master_courses/blueprint_associations_spec.rb index 42aa2f49b65..ed02a31b491 100644 --- a/spec/selenium/master_courses/blueprint_associations_spec.rb +++ b/spec/selenium/master_courses/blueprint_associations_spec.rb @@ -79,9 +79,9 @@ describe "Blueprint association settings" do it "course search dropdowns are populated", priority: "2", test_id: 3072438 do open_associations open_courses_list - select_boxes = ff('.bca-course-filter select') - expect(select_boxes[0]).to include_text("Default Term") - expect(select_boxes[1]).to include_text("sub account 0") + + expect(term_options).to include 'Default Term' + expect(sub_account_options).to include 'sub account 0' end end end diff --git a/spec/selenium/master_courses/course_picker_spec.rb b/spec/selenium/master_courses/course_picker_spec.rb index 586d36d9b40..d213f95b560 100644 --- a/spec/selenium/master_courses/course_picker_spec.rb +++ b/spec/selenium/master_courses/course_picker_spec.rb @@ -68,8 +68,8 @@ describe "master courses - course picker" do let(:course_search_input) {'.bca-course-filter input[type="search"]'} let(:filter_output) {'.bca-course-details__wrapper'} let(:loading) {'.bca-course-picker__loading'} - let(:term_filter) {'.bca-course-filter select:contains("Any Term")'} - let(:sub_account_filter) {'.bca-course-filter select:contains("Any Sub-Account")'} + let(:term_filter) {'#termsFilter'} + let(:sub_account_filter) {'#subAccountsFilter'} def wait_for_spinner begin @@ -130,7 +130,7 @@ describe "master courses - course picker" do get "/courses/#{@master.id}" open_associations open_courses_list - click_option(term_filter, 'fall term') + click_INSTUI_Select_option(term_filter, 'fall term') wait_for_spinner expect(available_courses().length).to eq(4) end @@ -139,7 +139,7 @@ describe "master courses - course picker" do get "/courses/#{@master.id}" open_associations open_courses_list - click_option(sub_account_filter, 'sub-account 1') + click_INSTUI_Select_option(sub_account_filter, 'sub-account 1') wait_for_spinner expect(available_courses().length).to eq(1) end