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.

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: inconsistent sidebar group toggle on navigation #182

Merged
merged 2 commits into from
Aug 25, 2020

Conversation

scmmishra
Copy link
Contributor

@scmmishra scmmishra commented Aug 25, 2020

PR Summary

This PR has a fix for inconsistent sidebar toggle for group items.

Previous Behaviour
After Fix

The problem

Unlike the previous problem (fixed here), this problem occurs when navigating between group items. From the buefy docs the behaviour that is intended is for only one group to be expanded at a time. This is not happening properly.

There is a peculiar collapse behaviour as seen below.

bad-collapse-1

Another peculiar behaviour

Solutions

The reason such a behaviour exists is that the state of the component is managed by our code using activeGroup. We watch the route and change the activeGroup based on the current route.

$route(to) {
// Set the current route name to true for active+expanded keys in the
// menu to pick up.
this.activeItem = { [to.name]: true };
if (to.meta.group) {
this.activeGroup = { [to.meta.group]: true };
}
},

However, on expand or collapse of a group, no event is triggered. The fix after figuring this out becomes pretty straightforward. BMenuItem has two UI "mutations", one is routing, the other is a toggle expand and collapse (both triggered by a click event). There is an event for route already, however there is no trigger on expand or collpase.

Buefy has a onClick handler for BMenuItem, which looks like the following

onClick(event) {
	// Some JS magic Jimmy doesn't need to see
	this.$emit('update:expanded', this.newActive)
	if (menu && menu.activable) {
		this.newActive = true
		this.$emit('update:active', this.newActive)
	}
}

The this.newActive returns the current state of the group (whether it's expanded or collapsed) it is emitted via the update:active event, which we can listen on. This PR adds that event.

Depending on update:active is not the most elegant solution, since the event is not intended for this. The alternate is to completely override the onClick event for this, which defeats the purpose of using the component library in the first place.

Links

@knadh knadh merged commit eaba083 into knadh:master Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants