From ecbb5a49ab54128fb1adff60033bd51f973a4f39 Mon Sep 17 00:00:00 2001 From: Misha Moroshko Date: Tue, 9 Aug 2016 10:48:11 +1000 Subject: [PATCH] Always render itemsContainer (DOM restructure) --- demo/src/components/App/App.less | 2 +- .../App/components/Example6/Example6.js | 2 + .../App/components/Example7/Example7.js | 2 + .../App/components/Example8/Example8.js | 1 + .../App/components/Example9/Example9.js | 1 + demo/src/components/App/components/theme.less | 41 ++--- demo/src/index.html | 1 - src/Autowhatever.js | 160 +++++++++++------- src/ItemsList.js | 56 +----- test/helpers.js | 8 +- test/plain-list/Autowhatever.test.js | 26 ++- test/plain-list/AutowhateverApp.js | 1 + 12 files changed, 162 insertions(+), 139 deletions(-) diff --git a/demo/src/components/App/App.less b/demo/src/components/App/App.less index b814540..50ef695 100644 --- a/demo/src/components/App/App.less +++ b/demo/src/components/App/App.less @@ -1,5 +1,5 @@ .container { - font-family: 'Roboto', sans-serif; + font-family: Helvetica, Arial, sans-serif; margin-bottom: 300px; } diff --git a/demo/src/components/App/components/Example6/Example6.js b/demo/src/components/App/components/Example6/Example6.js index ab90928..cfdd88b 100644 --- a/demo/src/components/App/components/Example6/Example6.js +++ b/demo/src/components/App/components/Example6/Example6.js @@ -35,6 +35,8 @@ function mapDispatchToProps(dispatch) { dispatch(updateInputValue(exampleId, event.target.value)); }, onKeyDown: (event, { newFocusedSectionIndex, newFocusedItemIndex }) => { + event.preventDefault(); // Don't move the cursor to start/end + if (typeof newFocusedItemIndex !== 'undefined') { dispatch(updateFocusedItem(exampleId, newFocusedSectionIndex, newFocusedItemIndex)); } diff --git a/demo/src/components/App/components/Example7/Example7.js b/demo/src/components/App/components/Example7/Example7.js index 3ad73b7..3e2fb06 100644 --- a/demo/src/components/App/components/Example7/Example7.js +++ b/demo/src/components/App/components/Example7/Example7.js @@ -45,6 +45,8 @@ function mapDispatchToProps(dispatch) { dispatch(updateInputValue(exampleId, event.target.value)); }, onKeyDown: (event, { newFocusedSectionIndex, newFocusedItemIndex }) => { + event.preventDefault(); // Don't move the cursor to start/end + if (typeof newFocusedItemIndex !== 'undefined') { dispatch(updateFocusedItem(exampleId, newFocusedSectionIndex, newFocusedItemIndex)); } diff --git a/demo/src/components/App/components/Example8/Example8.js b/demo/src/components/App/components/Example8/Example8.js index 75ac51d..3e3d4dd 100644 --- a/demo/src/components/App/components/Example8/Example8.js +++ b/demo/src/components/App/components/Example8/Example8.js @@ -65,6 +65,7 @@ function mapDispatchToProps(dispatch) { switch (event.key) { case 'ArrowDown': case 'ArrowUp': + event.preventDefault(); // Don't move the cursor to start/end dispatch(updateFocusedItem(exampleId, newFocusedSectionIndex, newFocusedItemIndex)); break; diff --git a/demo/src/components/App/components/Example9/Example9.js b/demo/src/components/App/components/Example9/Example9.js index f1c3d57..3faa5f5 100644 --- a/demo/src/components/App/components/Example9/Example9.js +++ b/demo/src/components/App/components/Example9/Example9.js @@ -139,6 +139,7 @@ Example.propTypes = { focusedSectionIndex: PropTypes.number, focusedItemIndex: PropTypes.number, items: PropTypes.array.isRequired, + onChange: PropTypes.func.isRequired, onFocus: PropTypes.func.isRequired, onBlur: PropTypes.func.isRequired, diff --git a/demo/src/components/App/components/theme.less b/demo/src/components/App/components/theme.less index ec8edc2..7d0c6c3 100644 --- a/demo/src/components/App/components/theme.less +++ b/demo/src/components/App/components/theme.less @@ -9,7 +9,8 @@ width: 240px; height: 30px; padding: 10px 20px; - font-size: 18px; + font-size: 16px; + font-family: Helvetica, Arial, sans-serif; border: 1px solid @border-color; border-radius: @border-radius; box-sizing: content-box; @@ -25,21 +26,29 @@ } .itemsContainer { - position: relative; - top: -1px; - width: 280px; + display: none; + + .containerOpen & { + display: block; + position: relative; + top: -1px; + width: 280px; + border: 1px solid @border-color; + background-color: #fff; + font-size: 16px; + line-height: 1.25; + border-bottom-left-radius: @border-radius; + border-bottom-right-radius: @border-radius; + z-index: 2; + max-height: 260px; + overflow-y: auto; + } +} + +.itemsList { margin: 0; padding: 0; list-style-type: none; - border: 1px solid @border-color; - background-color: #fff; - font-size: 16px; - line-height: 1.25; - border-bottom-left-radius: @border-radius; - border-bottom-right-radius: @border-radius; - z-index: 2; - max-height: 260px; - overflow-y: auto; } .item { @@ -62,12 +71,6 @@ } } -.sectionItemsContainer { - margin: 0; - padding: 0; - list-style-type: none; -} - .highlight { color: #ee0000; font-weight: 400; diff --git a/demo/src/index.html b/demo/src/index.html index 5fd93fa..fb3cca3 100644 --- a/demo/src/index.html +++ b/demo/src/index.html @@ -6,7 +6,6 @@ - diff --git a/src/Autowhatever.js b/src/Autowhatever.js index 87fdc23..807b383 100644 --- a/src/Autowhatever.js +++ b/src/Autowhatever.js @@ -11,11 +11,11 @@ const defaultTheme = { containerOpen: 'react-autowhatever__container--open', input: 'react-autowhatever__input', itemsContainer: 'react-autowhatever__items-container', + itemsList: 'react-autowhatever__items-list', item: 'react-autowhatever__item', itemFocused: 'react-autowhatever__item--focused', sectionContainer: 'react-autowhatever__section-container', - sectionTitle: 'react-autowhatever__section-title', - sectionItemsContainer: 'react-autowhatever__section-items-container' + sectionTitle: 'react-autowhatever__section-title' }; export default class Autowhatever extends Component { @@ -62,16 +62,23 @@ export default class Autowhatever extends Component { constructor(props) { super(props); + this.focusedItem = null; + this.setSectionsItems(props); this.setSectionIterator(props); this.setTheme(props); this.onKeyDown = this.onKeyDown.bind(this); this.storeInputReference = this.storeInputReference.bind(this); - this.storeItemsListReference = this.storeItemsListReference.bind(this); + this.storeItemsContainerReference = this.storeItemsContainerReference.bind(this); + this.onFocusedItemChange = this.onFocusedItemChange.bind(this); this.getItemId = this.getItemId.bind(this); } + componentDidMount() { + this.ensureFocusedItemIsVisible(); + } + componentWillReceiveProps(nextProps) { if (nextProps.items !== this.props.items) { this.setSectionsItems(nextProps); @@ -86,6 +93,10 @@ export default class Autowhatever extends Component { } } + componentDidUpdate() { + this.ensureFocusedItemIsVisible(); + } + setSectionsItems(props) { if (props.multiSection) { this.sectionsItems = props.items.map(section => props.getSectionItems(section)); @@ -111,13 +122,16 @@ export default class Autowhatever extends Component { } } - // Needed only for testing - storeItemsListReference(itemsList) { - if (itemsList !== null) { - this.itemsList = itemsList; + storeItemsContainerReference(itemsContainer) { + if (itemsContainer !== null) { + this.itemsContainer = itemsContainer; } } + onFocusedItemChange(focusedItem) { + this.focusedItem = focusedItem; + } + getItemId(sectionIndex, itemIndex) { if (itemIndex === null) { return null; @@ -129,12 +143,6 @@ export default class Autowhatever extends Component { return `react-autowhatever-${id}-${section}-item-${itemIndex}`; } - getItemsContainerId() { - const { id } = this.props; - - return `react-autowhatever-${id}`; - } - renderSections() { if (this.allSectionsAreEmpty) { return null; @@ -146,46 +154,39 @@ export default class Autowhatever extends Component { renderSectionTitle, focusedSectionIndex, focusedItemIndex, itemProps } = this.props; - return ( -
- { - items.map((section, sectionIndex) => { - if (!shouldRenderSection(section)) { - return null; - } - - const keyPrefix = `react-autowhatever-${id}-`; - const sectionKeyPrefix = `${keyPrefix}section-${sectionIndex}-`; - - // `key` is provided by theme() - /* eslint-disable react/jsx-key */ - return ( -
- - -
- ); - /* eslint-enable react/jsx-key */ - }) - } -
- ); + return items.map((section, sectionIndex) => { + if (!shouldRenderSection(section)) { + return null; + } + + const keyPrefix = `react-autowhatever-${id}-`; + const sectionKeyPrefix = `${keyPrefix}section-${sectionIndex}-`; + + // `key` is provided by theme() + /* eslint-disable react/jsx-key */ + return ( +
+ + +
+ ); + /* eslint-enable react/jsx-key */ + }); } renderItems() { @@ -203,16 +204,15 @@ export default class Autowhatever extends Component { return ( + keyPrefix={`react-autowhatever-${id}-`} /> ); } @@ -235,31 +235,73 @@ export default class Autowhatever extends Component { } } + ensureFocusedItemIsVisible() { + const { focusedItem } = this; + + if (!focusedItem) { + return; + } + + const { itemsContainer } = this; + const itemOffsetRelativeToContainer = + focusedItem.offsetParent === itemsContainer + ? focusedItem.offsetTop + : focusedItem.offsetTop - itemsContainer.offsetTop; + + let { scrollTop } = itemsContainer; // Top of the visible area + + if (itemOffsetRelativeToContainer < scrollTop) { + // Item is off the top of the visible area + scrollTop = itemOffsetRelativeToContainer; + } else if (itemOffsetRelativeToContainer + focusedItem.offsetHeight > scrollTop + itemsContainer.offsetHeight) { + // Item is off the bottom of the visible area + scrollTop = itemOffsetRelativeToContainer + focusedItem.offsetHeight - itemsContainer.offsetHeight; + } + + if (scrollTop !== itemsContainer.scrollTop) { + itemsContainer.scrollTop = scrollTop; + } + } + render() { const { theme } = this; const { id, multiSection, focusedSectionIndex, focusedItemIndex } = this.props; const renderedItems = multiSection ? this.renderSections() : this.renderItems(); const isOpen = (renderedItems !== null); const ariaActivedescendant = this.getItemId(focusedSectionIndex, focusedItemIndex); + const containerProps = theme( + `react-autowhatever-${id}-container`, + 'container', + isOpen && 'containerOpen' + ); + const itemsContainerId = `react-autowhatever-${id}`; const inputProps = { type: 'text', value: '', autoComplete: 'off', role: 'combobox', 'aria-autocomplete': 'list', - 'aria-owns': this.getItemsContainerId(), + 'aria-owns': itemsContainerId, 'aria-expanded': isOpen, + 'aria-haspopup': isOpen, 'aria-activedescendant': ariaActivedescendant, ...theme(`react-autowhatever-${id}-input`, 'input'), ...this.props.inputProps, onKeyDown: this.props.inputProps.onKeyDown && this.onKeyDown, ref: this.storeInputReference }; + const itemsContainerProps = { + id: itemsContainerId, + ...theme(`react-autowhatever-${id}-items-container`, 'itemsContainer'), + ref: this.storeItemsContainerReference + }; return ( -
+
- {renderedItems} +
+ {renderedItems} +
); } diff --git a/src/ItemsList.js b/src/ItemsList.js index 355ead5..78d39c4 100644 --- a/src/ItemsList.js +++ b/src/ItemsList.js @@ -4,7 +4,6 @@ import compareObjects from './compareObjects'; export default class ItemsList extends Component { static propTypes = { - id: PropTypes.string.isRequired, items: PropTypes.array.isRequired, itemProps: PropTypes.oneOfType([ PropTypes.object, @@ -14,6 +13,7 @@ export default class ItemsList extends Component { renderItemData: PropTypes.object.isRequired, sectionIndex: PropTypes.number, focusedItemIndex: PropTypes.number, + onFocusedItemChange: PropTypes.func.isRequired, getItemId: PropTypes.func.isRequired, theme: PropTypes.func.isRequired, keyPrefix: PropTypes.string.isRequired @@ -26,75 +26,27 @@ export default class ItemsList extends Component { constructor() { super(); - this.storeItemsContainerReference = this.storeItemsContainerReference.bind(this); this.storeFocusedItemReference = this.storeFocusedItemReference.bind(this); } - componentDidMount() { - this.ensureFocusedItemIsVisible(); - } - shouldComponentUpdate(nextProps) { return compareObjects(nextProps, this.props, ['itemProps']); } - componentDidUpdate() { - this.ensureFocusedItemIsVisible(); - } - - storeItemsContainerReference(itemsContainer) { - if (itemsContainer !== null) { - this.itemsContainer = itemsContainer; - } - } - storeFocusedItemReference(focusedItem) { - if (focusedItem !== null) { - this.focusedItem = focusedItem.item; - } - } - - ensureFocusedItemIsVisible() { - if (!this.focusedItem) { - return; - } - - const { focusedItem, itemsContainer } = this; - const itemOffsetRelativeToContainer = - focusedItem.offsetParent === itemsContainer - ? focusedItem.offsetTop - : focusedItem.offsetTop - itemsContainer.offsetTop; - - let { scrollTop } = itemsContainer; // Top of the visible area - - if (itemOffsetRelativeToContainer < scrollTop) { - // Item is off the top of the visible area - scrollTop = itemOffsetRelativeToContainer; - } else if (itemOffsetRelativeToContainer + focusedItem.offsetHeight > scrollTop + itemsContainer.offsetHeight) { - // Item is off the bottom of the visible area - scrollTop = itemOffsetRelativeToContainer + focusedItem.offsetHeight - itemsContainer.offsetHeight; - } - - if (scrollTop !== itemsContainer.scrollTop) { - itemsContainer.scrollTop = scrollTop; - } + this.props.onFocusedItemChange(focusedItem === null ? null : focusedItem.item); } render() { const { - id, items, itemProps, renderItem, renderItemData, sectionIndex, + items, itemProps, renderItem, renderItemData, sectionIndex, focusedItemIndex, getItemId, theme, keyPrefix } = this.props; const sectionPrefix = (sectionIndex === null ? keyPrefix : `${keyPrefix}section-${sectionIndex}-`); - const itemsContainerClass = (sectionIndex === null ? 'itemsContainer' : 'sectionItemsContainer'); const isItemPropsFunction = (typeof itemProps === 'function'); return ( -
    +
      { items.map((item, itemIndex) => { const isFocused = (itemIndex === focusedItemIndex); diff --git a/test/helpers.js b/test/helpers.js index 0bf08d4..8640c5f 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -16,11 +16,9 @@ export const init = application => { export const eventMatcher = sinon.match.instanceOf(SyntheticEvent); -export const getStoredFocusedItemName = () => { - const { focusedItem } = app.autowhatever.itemsList; - - return focusedItem ? focusedItem.constructor.name : null; -}; +export const getStoredInput = () => app.autowhatever.input; +export const getStoredItemsContainer = () => app.autowhatever.itemsContainer; +export const getStoredFocusedItem = () => app.autowhatever.focusedItem; export const getInputAttribute = attr => input.getAttribute(attr); diff --git a/test/plain-list/Autowhatever.test.js b/test/plain-list/Autowhatever.test.js index 597c84c..7dccc87 100644 --- a/test/plain-list/Autowhatever.test.js +++ b/test/plain-list/Autowhatever.test.js @@ -3,7 +3,9 @@ import TestUtils from 'react-addons-test-utils'; import { expect } from 'chai'; import { init, - getStoredFocusedItemName, + getStoredInput, + getStoredItemsContainer, + getStoredFocusedItem, getInputAttribute, getItemsContainerAttribute, getItems, @@ -34,6 +36,18 @@ describe('Plain List Autowhatever', () => { it('should call `renderItem` exactly `items.length` times', () => { expect(renderItem).to.have.callCount(5); }); + + it('should store the input on the instance', () => { + expect(getStoredInput().getAttribute('id')).to.equal('my-fancy-input'); + }); + + it('should store the items container on the instance', () => { + expect(getStoredItemsContainer().getAttribute('id')).to.equal('react-autowhatever-my-fancy-component'); + }); + + it('should set the stored focused item on the instance to null', () => { + expect(getStoredFocusedItem()).to.equal(null); + }); }); describe('hovering items', () => { @@ -68,7 +82,15 @@ describe('Plain List Autowhatever', () => { it('should store the focused item on the instance', () => { mouseEnterItem(2); - expect(getStoredFocusedItemName()).to.equal('HTMLLIElement'); + expect(getStoredFocusedItem().getAttribute('id')) + .to.equal('react-autowhatever-my-fancy-component--item-2'); + + mouseLeaveItem(2); + expect(getStoredFocusedItem()).to.equal(null); + + mouseEnterItem(3); + expect(getStoredFocusedItem().getAttribute('id')) + .to.equal('react-autowhatever-my-fancy-component--item-3'); }); }); }); diff --git a/test/plain-list/AutowhateverApp.js b/test/plain-list/AutowhateverApp.js index cb8859e..bfde38b 100644 --- a/test/plain-list/AutowhateverApp.js +++ b/test/plain-list/AutowhateverApp.js @@ -56,6 +56,7 @@ export default class AutowhateverApp extends Component { render() { const { value, focusedItemIndex } = this.state; const inputProps = { + id: 'my-fancy-input', value, onChange: this.onChange };