Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(sidebar): only expand sidebar when item has children elements #158

Merged
merged 3 commits into from
Jan 20, 2018
Merged

fix(sidebar): only expand sidebar when item has children elements #158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7ced12c
fix(sidebar): only expand sidebar when item has children elements
nnixaa Jan 20, 2018
2596b41
Merge branch 'master' into feature/fix-sidebar
nnixaa Jan 20, 2018
17e121f
Merge branch 'master' into feature/fix-sidebar
nnixaa Jan 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions e2e/menu.e2e-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ const homeButton = by.css('#homeBtn');

const waitTime = 20 * 1000;

const sidebarMenu31 = by.css('#menu-sidebar ul li:nth-child(4) ul li:nth-child(1) a > span');
const sidebarMenu31 = by.css('#menu-sidebar ul li:nth-child(4) ul li:nth-child(1) > a > span');
const sidebarMenu1 = by.css('#menu-sidebar ul li:nth-child(2) a');
const sidebarMenu3 = by.css('#menu-sidebar ul li:nth-child(4) a');

describe('nb-menu', () => {

Expand All @@ -42,7 +44,7 @@ describe('nb-menu', () => {
});

it('should display menu', () => {
expect(element(by.css('nb-menu')).isDisplayed()).toBeTruthy();
expect(element(by.css('#menu-first')).isDisplayed()).toBeTruthy();
expect(browser.getCurrentUrl()).toContain('#/menu/1');
});

Expand Down Expand Up @@ -103,6 +105,20 @@ describe('nb-menu', () => {
.getCssValue('display').then(value => expect(value).toEqual('block'));
});

it('should not expand sidebar when item has no children', () => {
element.all(sidebarMenu1).first().click()
.then(() => {
expect(hasClass(element.all(by.css('nb-sidebar')).first(), 'compacted')).toBeTruthy();
});
});

it('should expand sidebar when item has children', () => {
element.all(sidebarMenu3).first().click()
.then(() => {
expect(hasClass(element.all(by.css('nb-sidebar')).first(), 'expanded')).toBeTruthy();
});
});

it('should be selected - Menu #3.1', () => {
expect(hasClass(element.all(menu3SubMenu).first(), 'collapsed')).toBeTruthy();

Expand Down
15 changes: 14 additions & 1 deletion src/framework/theme/components/sidebar/sidebar.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,23 @@ export class NbSidebarComponent implements OnInit, OnDestroy {
}
}

// TODO: this is more of a workaround, should be a better way to make components communicate to each other
onClick(event): void {
const menu = this.element.nativeElement.querySelector('nb-menu');

if (menu && menu.contains(event.target)) {
this.expand();
let link = event.target;
const linkChildren = ['span', 'i'];

// if we clicked on span - get the link
if (linkChildren.indexOf(link.tagName.toLowerCase()) !== -1 && link.parentNode) {
link = event.target.parentNode;
}

// we only expand if an item has children
if (link && link.nextElementSibling && link.nextElementSibling.classList.contains('menu-items')) {
this.expand();
}
}
}

Expand Down