feat: add root heading level support to dashboard layout #9249
Conversation
| this.__removeHeadingLevelObserver(); | ||
| const parentLayout = getParentLayout(this); | ||
| if (parentLayout) { | ||
| this.__headingLevelObserver = new MutationObserver(() => this.__updateRootHeadingLevel()); |
There was a problem hiding this comment.
Should we just fire an event when the heading level changes? MIght be a bit more light-weight than creating a mutation observer per item instance.
There was a problem hiding this comment.
@ugur-vaadin Did you plan to address this?
There was a problem hiding this comment.
Updated the PR to use an event instead of a MutationObserver.
| */ | ||
| export function getParentLayout(node) { | ||
| return __findFilteredAncestorInstance(node, (el) => { | ||
| return el.constructor && el.constructor.toString().includes('DashboardLayoutMixin'); |
There was a problem hiding this comment.
This probably won't work in production where function and class names get minified. Did a quick test in a Flow project and "DashboardLayoutMixin" does not show up in any generated chunks. Since there are only two types of elements to test for, we could use the is getter to check for the tag name:
web-components/packages/dashboard/src/vaadin-dashboard.js
Lines 110 to 112 in 8ecd313
There was a problem hiding this comment.
Updated the helper to use is.
There was a problem hiding this comment.
@web-padawan Since you noted this in the other PR, should we prefer localName over is?
ugur-vaadin
force-pushed
the
feat-add-root-level-heading-support-to-dashboard-layout
branch
from
a0088af to
e9590b4
Compare
Co-authored-by: Serhii Kulykov <iamkulykov@gmail.com>
Co-authored-by: Serhii Kulykov <iamkulykov@gmail.com>
|
Description
This PR adds root heading level support to
DashboardLayout. It was previously added for Dashboard. This PR also refactors the heading level logic inDashboard,DashboardWidget, andDashboardSection; centralizing them inDashboardLayoutMixinandDashboardItemMixin.Fixes #8924
Type of change
Checklist