Skip to content

docs(navigation-drawer): remove nav tag since it is added from the component itself#5989

Merged
kdinev merged 3 commits intovnextfrom
mpopov/update-navdrawer-docs
Apr 1, 2026
Merged

docs(navigation-drawer): remove nav tag since it is added from the component itself#5989
kdinev merged 3 commits intovnextfrom
mpopov/update-navdrawer-docs

Conversation

@desig9stein
Copy link
Copy Markdown
Contributor

Related to IgniteUI/igniteui-angular#14847

Checklist:

  • check topic's TOC/menu and paragraph headings
  • Include TOC topic labels in topic content has a valuable update, it's new or considered as preview\ beta
  • link to other topics using ../relative/path.md
  • at the References section at the end of the topic add links to topics, samples, etc
  • reference API documentation instead of adding a section with API

  • use valid component names - [Data] Grid, IgxSelectComponent, <igx-combo>
  • use spell checker tool (VS Code, Grammarly, Microsoft Editor)
  • add inline code blocks for the names of classes / tags / properties
  • add language descriptor for the code blocks
  • check broken links (use browser add-on)
  • check if sample is working and fully visible in the topic
  • check if sample is working and fully visible in the StackBlitz
  • check if code blocks match the code in StackBlitz demo


  • do not resolve requested changes (leave that to the reviewer)
  • add pending-localization label when the review of the PR is done
  • add a member from the localization team to translate it

# Conflicts:
#	en/components/navdrawer.md
#	jp/components/navdrawer.md
@kdinev kdinev changed the base branch from master to vnext April 1, 2026 12:57
Copilot AI review requested due to automatic review settings April 1, 2026 12:58
@kdinev kdinev merged commit bc80aee into vnext Apr 1, 2026
7 checks passed
@kdinev kdinev deleted the mpopov/update-navdrawer-docs branch April 1, 2026 13:00
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the Navigation Drawer documentation examples to avoid wrapping drawer content in an extra <nav> element when the component already provides the appropriate semantic container.

Changes:

  • Removes <nav> wrappers from several igxDrawer template examples in KR/EN/JP docs.
  • Updates KR router examples formatting to remove the <nav> wrapper around repeated drawer items.
  • Fixes JP reference list formatting at the end of the topic.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
kr/components/navdrawer.md Removes <nav> wrappers from drawer templates, including router examples.
jp/components/navdrawer.md Removes <nav> wrappers from examples and normalizes the final reference list formatting.
en/components/navdrawer.md Removes <nav> wrappers from the introductory examples (but one <nav> remains later in the Router section).
Comments suppressed due to low confidence (1)

jp/components/navdrawer.md:354

  • Prefer property binding for routerLink here as well (e.g., [routerLink]="item.link") instead of routerLink="{{item.link}}". This avoids interpolation in an attribute value and keeps the RouterLink directive input typed consistently.
    <span
      *ngFor="let item of componentLinks"
      routerLink="{{item.link}}"
      routerLinkActive
      #rla="routerLinkActive"

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 122 to 132
```html
<!-- app.component.html -->
<div class="content-wrap">
<igx-nav-drawer id="navigation" #drawer [isOpen]="true">
<ng-template igxDrawer>
<nav>
<span igxDrawerItem [isHeader]="true">Components</span>
<span *ngFor="let item of navItems" igxDrawerItem [active]="item.text === selected" igxRipple (click)="navigate(item)">
<igx-icon fontSet="material">{{ item.name }}</igx-icon>
<span>{{ item.text }}</span>
</span>
</nav>
</ng-template>
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

This doc removes the extra <nav> wrapper in the early examples, but there is still a <nav> wrapper later in the same file in the "Using Angular Router" example (around line 347). That leaves the topic inconsistent with the stated goal (and can reintroduce nested <nav> landmarks if the component already renders one). Please remove that remaining <nav> wrapper or clarify why it is required specifically in that section.

Copilot uses AI. Check for mistakes.
Comment on lines +238 to +242
<span *ngFor="let item of componentLinks" routerLink="{{item.link}}"
igxDrawerItem igxRipple
routerLinkActive="igx-nav-drawer__item--active" >
{{item.name}}
</span>
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

In Angular templates, prefer property binding for routerLink instead of string interpolation in an attribute. Using [routerLink]="item.link" (or an array form) avoids unnecessary string interpolation and keeps the directive input typed as intended.

Copilot uses AI. Check for mistakes.
Comment on lines +250 to +254
<span *ngFor="let item of componentLinks" routerLink="{{item.link}}"
routerLinkActive #rla="routerLinkActive"
igxDrawerItem igxRipple [active]="rla.isActive">
{{item.name}}
</span>
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

Same as above: use property binding for routerLink (e.g., [routerLink]="item.link") rather than routerLink="{{item.link}}" to keep the RouterLink input typed and avoid interpolation in attribute values.

Copilot uses AI. Check for mistakes.
@github-actions github-actions Bot mentioned this pull request Apr 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants