Skip to content

Section header#52

Open
edlu77 wants to merge 15 commits into
mainfrom
section-link
Open

Section header#52
edlu77 wants to merge 15 commits into
mainfrom
section-link

Conversation

@edlu77
Copy link
Copy Markdown
Collaborator

@edlu77 edlu77 commented May 29, 2026

@edlu77 edlu77 requested review from LibbyUX and axdanbol May 29, 2026 21:08
@edlu77 edlu77 self-assigned this May 29, 2026
@nx-cloud
Copy link
Copy Markdown
Contributor

nx-cloud Bot commented May 29, 2026

View your CI Pipeline Execution ↗ for commit c00eabb

Command Status Duration Result
nx affected -t lint test build build-compodoc b... ✅ Succeeded 1m 19s View ↗
nx-cloud record -- nx format:check ✅ Succeeded 2s View ↗
nx-cloud record -- commitlint --from 279c374f39... ✅ Succeeded <1s View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-06-05 16:16:38 UTC

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 29, 2026

🚀 Preview Deploy Report

✅ Successfully deployed preview here

Copy link
Copy Markdown
Contributor

@nx-cloud nx-cloud Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

At least one additional CI pipeline execution has run since the conclusion below was written and it may no longer be applicable.

Nx Cloud is proposing a fix for your failed CI:

We corrected the styleIncludePaths in ng-package.json from ../../../internal/scss to ../../internal/scss, aligning it with the actual directory depth of the section-link library under content-templates/. This fixes the build error where the Angular compiler could not resolve the token-utils SCSS import because the path was pointing one level too high into a non-existent directory.

Tip

We verified this fix by re-running design-system:build:production.

diff --git a/libs/design-system/content-templates/section-link/ng-package.json b/libs/design-system/content-templates/section-link/ng-package.json
index 49b7b20..27b8836 100644
--- a/libs/design-system/content-templates/section-link/ng-package.json
+++ b/libs/design-system/content-templates/section-link/ng-package.json
@@ -1,6 +1,6 @@
 {
   "lib": {
     "entryFile": "src/index.ts",
-    "styleIncludePaths": ["../../../internal/scss"]
+    "styleIncludePaths": ["../../internal/scss"]
   }
 }

🔔 Heads up, your workspace has pending recommendations ↗ to auto-apply fixes for similar failures.

Apply fix via Nx Cloud  Reject fix via Nx Cloud


Or Apply changes locally with:

npx nx-cloud apply-locally mU9S-cvjU

Apply fix locally with your editor ↗   View interactive diff ↗



🎓 Learn more about Self-Healing CI on nx.dev

Copy link
Copy Markdown
Collaborator

@LibbyUX LibbyUX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @edlu77 ! This looks good.

Just a couple questions:

  • Is it possible to default to using the divider for this component? Asking because I am moving towards always using them myself because they break up the page nicely.
  • If possible, can we use the default styles for this component? Details on a previous review I wrote earlier today: #45 (review)

Thanks so much, talk soon!

@LibbyUX LibbyUX linked an issue Jun 1, 2026 that may be closed by this pull request
1 task
@LibbyUX
Copy link
Copy Markdown
Collaborator

LibbyUX commented Jun 2, 2026

@edlu77 - This may still be a work-in-progress, so please disregard if so!

On Brave browser, I am seeing the icon button always showing with alignment issues with the icon in the icon button, and also with the alignment of the header and the icon button.

image image

@edlu77 edlu77 changed the title Section link Section header Jun 2, 2026
@LibbyUX LibbyUX added this to the WPP MVP1 milestone Jun 4, 2026
Copy link
Copy Markdown
Member

@axdanbol axdanbol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move code from design-system/content-templates/section-header/ to design-system/section-header/ & also add an entry in tsconfig.base.json

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Write proper tests using testing-library

argTypes: {
level: {
control: { type: 'number', min: 1, max: 6 },
description: 'The heading level (1-6) to determine the appropriate HTML tag.',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

render: (args) => ({
props: args,
styles: ['[ang-section-header] { margin: 0 2rem; }'],
template: `<h${clampLevel(args.level)} ang-section-header anchor="${args.anchor}" underlined="${args.underlined}">
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use ${argsToTemplate(args, ...)} instead of binding anchor and underlined

@Component({
selector:
// eslint-disable-next-line @angular-eslint/component-selector
`h1[ang-section-header], h2[ang-section-header], h3[ang-section-header],
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this uses attribute style selectors we should update each to hX[angSectionHeader]

@@ -0,0 +1,15 @@
@if (anchor()) {
<span class="ang-section-header-link-container">
<a mat-icon-button class="ang-section-header-link" aria-label="Link to this heading" [attr.href]="`#${anchor()}`">
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to use aria-labelledby instead of aria-label. You need to set a unique id on the ang-section-header-content div

@LibbyUX LibbyUX self-requested a review June 5, 2026 20:45
Copy link
Copy Markdown
Collaborator

@LibbyUX LibbyUX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much @edlu77! Have a great weekend!

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.

Add section header

3 participants