-
Notifications
You must be signed in to change notification settings - Fork 49
feat: add tab overflow menu (#1592) #1682
base: main
Are you sure you want to change the base?
Conversation
trusz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution!
The solution looks nice and I would like to ask for a few changes to finalize the solution. You can of course let us know if we should take over from here.
- I have small code change request about adding and removing event listeners.
- Also, I was wondering if you could place the overflow menu at the and of the row and not at the beginning. I think they are usually there.
- The style of the selected tabs have changed a bit. Would it be possible to revert them how they've looked like?
- I think it would be nice somehow to display the currently active plugin even if it in the overflow menu, but currently I do not have an idea how. I am open to suggestions.
| } | ||
|
|
||
| disconnectedCallback() { | ||
| window.removeEventListener('resize', () => this.#calculateVisibleTabs()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this won't remove the original one.
I would recommend creating a function that we can reference.
something along these lines:
private calculateVisibleTabs = () => {
...
}By using an arrow function we make sure that we have the correct context (this) without binding the function.
With that you can add and remove listeners:
window.addEventListener('resize', this.calculateVisibleTabs)
window.RemoveEventListener('resize', this.calculateVisibleTabs)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Thanks for the hint that removing the resize listener did not work. I fixed it now.
- The overflow menu is now placed at the end of the row - I like this solution better as well.
- I find styling Material Web Components quite painful, because there are many fixed/internal styles. I've adjusted the buttons to look more like mwc-tab-bar, but I couldn’t increase the icon size or vertical padding. Any ideas?
- My idea for improving the UI: If the active plugin is hidden in the overflow menu, move its button to the right-most visible position, and move the button currently there into the menu instead. This could be repeated if needed (e.g. if the active button is wider).
closes #1592