Skip to content

Refactor setting management to use @angular/aria tabs - issue-24565 #24623

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.

Sign up for GitHub

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

Merged
oykuermann merged 2 commits into from
Jan 19, 2026
Merged

Refactor setting management to use @angular/aria tabs - issue-24565 #24623

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
14f284c
Refactor setting management to use @angular/aria tabs
fahrigedik Jan 12, 2026
ca839db
Add @angular/aria dependency to Angular packages
fahrigedik Jan 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion npm/ng-packs/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
"@angular-eslint/eslint-plugin-template": "~21.0.0",
"@angular-eslint/template-parser": "~21.0.0",
"@angular/animations": "21.0.0",
"@angular/aria": "~21.0.0",
"@angular/build": "~21.0.0",
"@angular/cli": "~21.0.0",
"@angular/common": "~21.0.0",
Expand Down Expand Up @@ -147,4 +148,4 @@
"dependencies": {
"openid-client": "^6.6.4"
}
}
}
38 changes: 13 additions & 25 deletions ...ng-packs/packages/setting-management/src/lib/components/setting-management.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,41 +2,29 @@
<div id="SettingManagementWrapper">
<div class="card">
<div class="card-body">
<div class="row">
<div class="row" ngTabs>
<div class="col-12 col-md-3 mb-2 mb-md-0">
<ul class="nav flex-column nav-pills" id="nav-tab" role="tablist">
<div class="nav flex-column nav-pills" id="nav-tab" ngTabList orientation="vertical"
[selectedTab]="selected?.name">
<ng-container *abpFor="let setting of settings; trackBy: trackByFn">
<li
(click)="selected = setting"
class="nav-item pointer"
*abpPermission="setting.requiredPolicy"
>
<a
class="nav-link"
[id]="setting.name + '-tab'"
role="tab"
[class.active]="setting.name === selected.name"
>{{ setting.name | abpLocalization }}</a
>
</li>
<button ngTab #tab="ngTab" [value]="setting.name" (click)="selected = setting"
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

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

The (click)="selected = setting" handler may be redundant since @angular/aria's Tab component should manage tab selection internally via the [selectedTab]="selected?.name" binding on the TabList. If the @angular/aria tabs properly sync with the selectedTab input, the manual click handler may cause unnecessary state updates. Consider removing the click handler and letting @angular/aria manage the selection state, or if manual tracking is needed, consider using a change event from the @angular/aria tabs instead.

Suggested change
<button ngTab #tab="ngTab" [value]="setting.name" (click)="selected = setting"
<button ngTab #tab="ngTab" [value]="setting.name"

Copilot uses AI. Check for mistakes.
class="nav-link text-start" [class.active]="tab.selected()" *abpPermission="setting.requiredPolicy">
{{ setting.name | abpLocalization }}
</button>
</ng-container>
</ul>
</div>
</div>
<div class="col-12 col-md-9">
@if (settings.length) {
<div class="tab-content pt-0">
<div
class="tab-pane fade show active"
[id]="selected.name + '-tab'"
role="tabpanel"
>
<ng-container *ngComponentOutlet="selected.component" />
</div>
<ng-container *abpFor="let setting of settings; trackBy: trackByFn">
<div ngTabPanel [value]="setting.name" class="tab-pane" *abpPermission="setting.requiredPolicy">
<ng-container *ngComponentOutlet="setting.component" />
Comment on lines +20 to +21
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

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

All tab panels are being rendered in the DOM regardless of whether they are selected, with visibility controlled by the [inert] attribute and CSS display: none. This means that all setting components are instantiated and initialized when the page loads, which could impact performance if there are many tabs or if the components are expensive to initialize. Consider whether @angular/aria supports lazy rendering of tab panels, or if the current approach is acceptable given the typical number of setting tabs.

Suggested change
<div ngTabPanel [value]="setting.name" class="tab-pane" *abpPermission="setting.requiredPolicy">
<ng-container *ngComponentOutlet="setting.component" />
<div ngTabPanel [value]="setting.name" class="tab-pane" *abpPermission="setting.requiredPolicy">
@if (selected?.name === setting.name) {
<ng-container *ngComponentOutlet="setting.component" />
}

Copilot uses AI. Check for mistakes.
</div>
</ng-container>
}
</div>
</div>
</div>
</div>
</div>
</abp-page>
</abp-page>
8 changes: 7 additions & 1 deletion npm/ng-packs/packages/setting-management/src/lib/components/setting-management.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,17 @@ import { Component, inject, OnDestroy, OnInit, TrackByFunction } from '@angular/
import { Subscription } from 'rxjs';
import { NgComponentOutlet } from '@angular/common';
import { PageComponent } from '@abp/ng.components/page';
import { Tab, Tabs, TabList, TabPanel } from '@angular/aria/tabs';

@Component({
selector: 'abp-setting-management',
templateUrl: './setting-management.component.html',
imports: [NgComponentOutlet, PageComponent, LocalizationPipe, PermissionDirective, ForDirective],
imports: [NgComponentOutlet, PageComponent, LocalizationPipe, PermissionDirective, ForDirective, Tabs, TabList, Tab, TabPanel],
styles: [`
:host [ngTabPanel][inert] {
display: none;
}
`],
})
export class SettingManagementComponent implements OnDestroy, OnInit {
private settingTabsService = inject(SettingTabsService);
Expand Down
Loading