-
Notifications
You must be signed in to change notification settings - Fork 11
feat: make page header configurable to persist active tab #1147
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.
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
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1147 +/- ##
==========================================
- Coverage 85.05% 85.02% -0.03%
==========================================
Files 829 829
Lines 17152 17175 +23
Branches 2228 2233 +5
==========================================
+ Hits 14588 14603 +15
- Misses 2533 2540 +7
- Partials 31 32 +1
Continue to review full report at Codecov.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
) {} | ||
|
||
public ngOnInit(): void { | ||
this.getPreferences().subscribe(preferences => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to unsubscribe this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
…nally-persist-page-header-active-tab
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@@ -57,7 +68,8 @@ export class NavigableTabGroupComponent implements AfterContentInit { | |||
public ngAfterContentInit(): void { | |||
this.activeTab$ = merge(this.navigationService.navigation$, this.tabs.changes).pipe( | |||
startWith(undefined), | |||
map(() => this.findActiveTab()) | |||
map(() => this.findActiveTab()), | |||
tap(activeTab => this.tabChange.emit(activeTab?.path)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this would emit duplicate events. May be we can put distinctUntilChanged
operator before the tap.
…nally-persist-page-header-active-tab
Description
This allows consumers of the page header to provide a unique persitanceId that will turn on saving the active tab to browser storage.