-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[Draft][Do not merge]Convert sidebar from paper-listbox to mwc-list #7326
[Draft][Do not merge]Convert sidebar from paper-listbox to mwc-list #7326
Conversation
Needed before merging
|
${this.editMode && this._hiddenPanels.length | ||
? html` | ||
${this._hiddenPanels.map((url) => { | ||
const panel = this.hass.panels[url]; | ||
if (!panel) { | ||
return ""; | ||
} | ||
return html`<paper-icon-item | ||
return html`<mwc-list-item |
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.
Should use graphic="icon"
here
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.
Then update the slot of the item to slot="graphic
@click=${this._handleExternalAppConfiguration} | ||
@mouseenter=${this._itemMouseEnter} | ||
@mouseleave=${this._itemMouseLeave} | ||
> | ||
<ha-svg-icon | ||
slot="item-icon" |
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.
slot="item-icon" | |
slot="graphic" |
@mouseleave=${this._itemMouseLeave} | ||
> | ||
<paper-icon-item> | ||
<mwc-list-item> |
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.
Should use graphic="icon"
here
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.
<mwc-list-item> | |
<mwc-list-item |
<ha-svg-icon slot="graphic" .path=${mdiBell}></ha-svg-icon> | ||
${!this.expanded && notificationCount > 0 | ||
? html` | ||
<span class="notification-badge" slot="graphic"> |
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.
<span class="notification-badge" slot="graphic"> | |
<span class="notification-badge"> |
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.
Does this work? not sure
: ""} | ||
${hass.localize("ui.notification_drawer.title")} | ||
${this.expanded && notificationCount > 0 | ||
? html` <span class="notification-badge">${notificationCount}</span> ` |
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.
? html` <span class="notification-badge">${notificationCount}</span> ` | |
? html`<span class="notification-badge">${notificationCount}</span>` |
></ha-user-badge> | ||
${hass.user ? hass.user.name : ""} | ||
</mwc-list-item> | ||
<mwc-list-item disabled class="bottom-spacer"></mwc-list-item> |
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.
If you use a divider does it produce the same result?
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 saw that, but I assumed that dividers were just the little <hr />
lines inbetween list items. I'll play with that.
class="ha-scrollbar" | ||
.selected=${hass.panelUrl} |
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.
You should set activated
on the panel that is active
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.
> | ||
<paper-icon-item> | ||
<mwc-list-item> | ||
<a |
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.
The anchor should be outside of the item, you can no longer right click and open in a new tab now. Also keyboard navigation doesn't work now.
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.
Putting the anchor outside the item restores right-click, but arrowing down is still broken. It just scrolls down the sidebar but doesn't highlight any of the items.
Still looking into it, but nothing's obviously wrong. I may just need to start over from dev
as I'm not sure what all I changed in order to make the buttons work without the anchor tag
</paper-icon-item> | ||
</div> | ||
|
||
<a |
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.
The anchor should not be removed, it removes functionality
Closing this PR as the overall approach has changed radically. I've put up a new Draft PR for my own reference, and also so I can have early ad-hoc discussions with other devs when I need some help: #7573. At the time of this writing, it is not worth reviewing as it's full of junk and unused code. |
Proposed change
Change sidebar to use
mwc-list
instead ofpaper-listbox
.This fixes an issue with quick bar where shortcuts won't work if you click on the left nav at all (because iron behaviors have a bug where it stops propagation of keyboard events: PolymerElements/iron-menu-behavior#56)
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed: