Skip to content

Commit

Permalink
[Menu] [Popover] Fix Menu not anchored correctly if active item is de…
Browse files Browse the repository at this point in the history
…ep in the scroll view

Fix flow errors

Test method returning positioning styles; remove redundant tests
  • Loading branch information
quiaro committed Jul 6, 2017
1 parent e36547e commit af02a89
Show file tree
Hide file tree
Showing 4 changed files with 338 additions and 247 deletions.
50 changes: 44 additions & 6 deletions src/Menu/Menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,51 @@ class Menu extends Component<DefaultProps, Props, void> {
return false;
};

getContentAnchorEl = () => {
if (!this.menuList || !this.menuList.selectedItem) {
// $FlowFixMe
return findDOMNode(this.menuList).firstChild;
getContentAnchorOffset = (element, anchor) => {
const list = findDOMNode(this.menuList);
let topOffset = 0;
let selectedItem;

if (list) {
selectedItem =
this.menuList && this.menuList.selectedItem
? findDOMNode(this.menuList.selectedItem)
: list.firstChild;
}

return findDOMNode(this.menuList.selectedItem);
if (list && selectedItem && anchor) {
// $FlowFixMe
const itemHeight = selectedItem.clientHeight;
// $FlowFixMe
const numItems = Math.floor(list.clientHeight / itemHeight);
const numVisibleItems = Math.floor(element.clientHeight / itemHeight);
const itemsOffLimits = Math.floor(numVisibleItems / 2);
// $FlowFixMe
const selectedIdx = Math.floor((selectedItem.offsetTop + itemHeight) / itemHeight) - 1;
// $FlowFixMe
const listPadding = (list.clientHeight - numItems * itemHeight) / 2;

// Calculate scroll to vertically center the selected list item within the
// popover and calculate the popover's top offset based on the position of
// the selected item so that it aligns with the anchor element.
if (selectedIdx < itemsOffLimits || numItems <= numVisibleItems) {
// No scroll necessary
topOffset = selectedIdx * itemHeight + itemHeight / 2 + listPadding;
} else {
if (selectedIdx >= numItems - itemsOffLimits) { // eslint-disable-line
// Items at the end of the popover
element.scrollTop = (numItems - numVisibleItems) * itemHeight + listPadding / 2;
topOffset = selectedIdx + 1 - (numItems - itemsOffLimits);
topOffset = element.clientHeight / 2 + topOffset * itemHeight;
} else {
// Selected item will be scrolled to the middle of the popover
element.scrollTop = (selectedIdx - itemsOffLimits) * itemHeight + listPadding / 2;
topOffset = element.clientHeight / 2;
}
}
topOffset = anchor - topOffset;
}
return topOffset;
};

render() {
Expand All @@ -158,7 +196,7 @@ class Menu extends Component<DefaultProps, Props, void> {
return (
<Popover
anchorEl={anchorEl}
getContentAnchorEl={this.getContentAnchorEl}
getContentAnchorOffset={this.getContentAnchorOffset}
className={classNames(classes.root, className)}
open={open}
enteredClassName={classes.entered}
Expand Down
69 changes: 61 additions & 8 deletions src/Menu/Menu.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@ describe('<Menu />', () => {
);
});

it('should pass the instance function `getContentAnchorEl` to Popover', () => {
it('should pass the instance function `getContentAnchorOffset` to Popover', () => {
const wrapper = shallow(<Menu />);
assert.strictEqual(
wrapper.props().getContentAnchorEl,
wrapper.instance().getContentAnchorEl,
wrapper.props().getContentAnchorOffset,
wrapper.instance().getContentAnchorOffset,
'should be the same function',
);
});
Expand Down Expand Up @@ -111,9 +111,10 @@ describe('<Menu />', () => {
let wrapper;
let instance;

let selectedItemFocusSpy;
let menuListSpy;
let menuListFocusSpy;
let selectedItemSpy;
let selectedItemFocusSpy;

let elementForHandleEnter;

Expand All @@ -127,20 +128,20 @@ describe('<Menu />', () => {
wrapper = mount(<Menu.Naked classes={classes} />);
instance = wrapper.instance();

selectedItemFocusSpy = spy();
menuListFocusSpy = spy();
menuListSpy = {};
menuListSpy.clientHeight = MENU_LIST_HEIGHT;
menuListSpy.style = {};
menuListSpy.firstChild = {
focus: menuListFocusSpy,
};
selectedItemSpy = {};
selectedItemFocusSpy = spy();
selectedItemSpy.focus = selectedItemFocusSpy;

findDOMNodeStub = stub(ReactDOM, 'findDOMNode').callsFake(arg => {
if (arg === SELECTED_ITEM_KEY) {
return {
focus: selectedItemFocusSpy,
};
return selectedItemSpy;
}
return menuListSpy;
});
Expand Down Expand Up @@ -218,5 +219,57 @@ describe('<Menu />', () => {
assert.notStrictEqual(menuListSpy.style.width, undefined);
});
});

describe('with anchor alignment', () => {
const element = { clientHeight: 30, scrollTop: 0 };
const anchor = 100;

before(() => {
instance.menuList = {};
instance.menuList.selectedItem = SELECTED_ITEM_KEY;
});

it('should compute offset for Popover when selected item is at the top', () => {
selectedItemSpy = { clientHeight: 6, offsetTop: 0 };
const offset = instance.getContentAnchorOffset(element, anchor);
assert.strictEqual(offset, 95);
});

it('should compute offset for Popover when selected item is second from top', () => {
selectedItemSpy = { clientHeight: 6, offsetTop: 8 };
const offset = instance.getContentAnchorOffset(element, anchor);
assert.strictEqual(offset, 89);
});

it('should compute offset for Popover when selected item is in the middle', () => {
selectedItemSpy = { clientHeight: 6, offsetTop: 56 };
const offset = instance.getContentAnchorOffset(element, anchor);
assert.strictEqual(offset, 85);
});

it('should compute offset for Popover when selected item is second to last', () => {
selectedItemSpy = { clientHeight: 6, offsetTop: 86 };
const offset = instance.getContentAnchorOffset(element, anchor);
assert.strictEqual(offset, 79);
});

it('should compute offset for Popover when selected item is last', () => {
selectedItemSpy = { clientHeight: 6, offsetTop: 92 };
const offset = instance.getContentAnchorOffset(element, anchor);
assert.strictEqual(offset, 73);
});

it('should scroll Popover to make selected item in the middle visible', () => {
selectedItemSpy = { clientHeight: 6, offsetTop: 56 };
instance.getContentAnchorOffset(element, anchor);
assert.strictEqual(element.scrollTop, 43);
});

it('should scroll Popover to make selected item at the bottom visible', () => {
selectedItemSpy = { clientHeight: 6, offsetTop: 92 };
instance.getContentAnchorOffset(element, anchor);
assert.strictEqual(element.scrollTop, 67);
});
});
});
});
Loading

0 comments on commit af02a89

Please sign in to comment.