fix(element-ng): migrate internal list-group usages to element list - #2746
Open
kfenner wants to merge 1 commit into
Open
fix(element-ng): migrate internal list-group usages to element list#2746kfenner wants to merge 1 commit into
kfenner wants to merge 1 commit into
Conversation
Replace the deprecated Bootstrap-derived list group (.list-group / .list-group-item) with the Element list (.list / .list-item) in the si-about and si-empty-state library components and in the drag-drop example. Final layer of the list-group deprecation stack (#2742, #2745). This re-renders the internal DOM of these shipped components. In si-about the link rows are now `ul.list.list-divider > li > a.list-item.list-item-action` and the branding block (icon, app name, subheadings, copyright) is a plain card block rather than a list row. si-empty-state's private wrapper class `.list-group-item-empty` is renamed to `.empty-state-content`. Public component inputs and outputs are unchanged. Typed `fix` rather than `refactor` on design drift: the deprecated list-group CSS no longer receives design-system updates, so components left on it diverge visually over time. `refactor` is also hidden from the changelog and triggers no release, so consumers would never see this note. NOTE: The internal markup of si-about and si-empty-state changed. Consumers with custom CSS or tests targeting `.list-group-item` inside these components must re-target: si-about link rows are now `ul.list.list-divider > li > a.list-item.list-item-action` (the branding area is now a plain card block, not a list row), and si-empty-state's wrapper element is `.empty-state-content`. Public inputs and outputs are unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Layer 3 of 3 — migrate remaining internal
list-groupusagesTop layer of the list-group deprecation stack. Base branch is
kaspar-fenner-cards-with-lists(layer 2), notmain.refactor(element-theme): deprecate list group(docs removed, SCSS deprecated in place, migration guide).feat(element-theme): support element list inside cards.This is not a class rename.
.list-group-itemisdisplay: block;.list-itemis a CSS grid with named slots (.list-item-title,.list-item-action, …) plus> * { grid-column: 3 / -1 }. Each component was restructured into those slots rather than having the class swapped.Per-component changes
si-about(projects/element-ng/about/).list-groupwas doing two unrelated jobs. Split into:<div class="about-branding">(icon/image, heading, subheadings, copyright notice), styled directly in SCSS; and<ul class="list list-divider">whose rows are the external links.<li><a class="list-item list-item-action text-primary" …><span class="list-item-title">…</span></a></li>.text-primaryis kept intentionally: it carries a load-bearing!importantthat beats.list-item-action'scolor: inherit, so link colour is unchanged..list-group-item(and a dead.with-border) replaced by.about-branding(padding + bottom border)..list-group-item:first-of-typeto.about-branding.si-empty-state(projects/element-ng/empty-state/).list-group-item-empty→.empty-state-contentin both HTML and SCSS. It was never a real list row (single element, no list semantics). Zero visual change.drag-dropexample (src/app/examples/drag-drop/)<ul class="list-group list-group-flush">→<ul class="list list-divider">; items →<li class="list-item list-item-action"><span class="list-item-title">…</span></li>.cdkDrag/cdkListbox/cdkOption/cdkDropList/cdkContextMenubindings and template refs preserved.Descoped (report only — deliberately left on
list-group)The dashboard widget families do not map cleanly and would need a design-system decision, so per the "you're allowed to descope" instruction they are left untouched:
si-widget-catalog(projects/dashboards-ng/…— html/scss/spec)si-list-widget-body/si-list-widget-item(projects/element-ng/dashboard/widgets/si-list-widget/…)custom-widget-catalog(dashboards-demo/…)Both gaps below are tracked in #2748 ("Element list: add selected state and preserve link colour on
.list-item-actionanchors"), framed as what blocks completing the list-group migration for these widgets.Headline reason: the Element list has no selected / current-item state. These widgets rely on a highlighted "selected" row (catalog selection, active list item).
.list-itemonly exposes&:active; there is no.active/selected equivalent, so a faithful migration would need a new design-system affordance rather than a markup change.Secondary gap:
.list-item-actionon an<a>defaults tocolor: inherit, so link-coloured rows need an explicit colour class (as done forsi-aboutviatext-primary, whose load-bearing!importantbeats that inherit). Both are captured in #2748 as prerequisites before these widgets migrate.Consumer-visible DOM change
This PR changes the rendered DOM of shipped components (
si-about,si-empty-state), so it is typedfix(notrefactor—refactorishiddenincommit-config.jsand would produce no release/changelog entry). The commit carries aNOTE:footer telling consumers to re-target any custom CSS/tests from.list-group-itemto the new markup (ul.list.list-divider > li > a.list-item.list-item-actionfor si-about links;.about-brandingfor the branding block;.empty-state-contentfor si-empty-state). Public inputs/outputs are unchanged, so this is not typedBREAKING CHANGE.Accessibility
Layer 2's card focus-ring bug (item background painting over the inset focus ring) was checked here and does not occur: in every migrated list the focusable element owns its own background (the
<a>in si-about, the<li>in drag-drop), so its inset outline paints above it. Verified visually — focus ring unbroken at rest and with the first row hovered. No localz-indexfix needed.VRT snapshots
Docker was unavailable in this environment, so VRT snapshots could not be regenerated.
si-aboutrenders differently and its snapshots will need updating:projects/element-ng/…/playwright/snapshots/si-about.spec.ts-snapshots/*(light + dark)si-about--si-about-api*,si-about--si-about-text*si-empty-state(private class rename) anddrag-dropare not expected to change visually. Please regeneratesi-aboutsnapshots with./e2e-local.sh update si-abouton a Docker-capable machine before merge.Verification run (mirrors CI
verificationjob)npx prettier --checkon all touched files — clean.pnpm run lint:scss— pass.ng lint element-ng+ng lint element-examples— pass (fulllint:ngOOMs on unrelateddashboards-demo-mfe, a pre-existing env issue).si-about+si-empty-statespecs — 11/11 pass.