feat(angular): use tabs without router #29786
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
github-actions
bot
added
package: core
thetaPC
requested review from
rugoncalves and
brandyscarney
and removed request for
a team
| @@ -39,8 +41,29 @@ export abstract class IonTabs implements AfterContentInit, AfterContentChecked { | |||
|
|
|||
| private tabBarSlot = 'bottom'; | |||
|
|
|||
| private hasTab = false; | |||
| private selectedTab?: any; | |||
There was a problem hiding this comment.
Using any here is overriding the type signature of treating it as optionally defined (undefined). This means you are missing out on potential errors on L203, L206 and L212.
I would recommend minimally updating the type signature to:
private selectedTab?: { tab: string };and then handling those type errors that arise for the selectedTab not being defined.
| } | ||
|
|
||
| if (leavingTab?.tab !== selectedTab.tab) { | ||
| if (leavingTab) { |
There was a problem hiding this comment.
I believe this should be checking that the el is defined, not the leavingTab, since it will based on the check on L206.
| } | ||
|
|
||
| private tabSwitch(): void { | ||
| const selectedTab = this.selectedTab; |
There was a problem hiding this comment.
~ These two lines could be minimized to:
const { selectedTab, leavingTab } = this;| @@ -11,11 +11,13 @@ import { IonRouterOutlet } from './ion-router-outlet'; | |||
| <ng-content select="[slot=top]"></ng-content> | |||
| <div class="tabs-inner" #tabsInner> | |||
| <ion-router-outlet | |||
| *ngIf="tabs?.length === 0" | |||
There was a problem hiding this comment.
Can tabs be undefined? The type signature implies not.
There was a problem hiding this comment.
After reviewing it further, it seems that there isn't a moment that it would be undefined. No issues were found when removing the optional chain. f6132ca
| @@ -11,11 +12,13 @@ import { IonRouterOutlet } from './router-outlet'; | |||
| <ng-content select="[slot=top]"></ng-content> | |||
| <div class="tabs-inner" #tabsInner> | |||
| <ion-router-outlet | |||
| *ngIf="tabs?.length === 0" | |||
There was a problem hiding this comment.
Similar feedback here if tabs can be undefined.
| /** | ||
| * Note: These must be redeclared on each child class since it needs | ||
| * access to generated components such as IonRouterOutlet and IonTabBar. | ||
| */ | ||
| abstract outlet: any; | ||
| abstract tabBar: any; | ||
| abstract tabBars: any; | ||
| abstract tabs: any; |
There was a problem hiding this comment.
Minimally the type signature should be:
abstract tabs: QueryList<any>;
Co-authored-by: Sean Perkins <[email protected]>
Co-authored-by: Sean Perkins <[email protected]>
There was a problem hiding this comment.
Tested the latest dev build in:
- A new Angular tabs starter app (NgModule)
- A new Angular tabs starter app (Standalone)
- A new Angular tabs starter app with
ion-router-outletremoved and<ion-tab>components added containing only text content (NgModule) - A new Angular tabs starter app with
ion-router-outletremoved and<ion-tab>components added containing only text content (Standalone) - The Ionic Angular Conference App
Everything is working correctly. Great job on this! 🎉
Issue number: part of #25184
This PR does not completely close the issue since it only addresses the Angular portion. The other frameworks will be done through other PRs.
What is the current behavior?
Angular: Developers are defaulted to have
ion-router-outletwhenion-tabsis in their app. This means that developers are forced to tie their tabs to a router.What is the new behavior?
ion-tabscan be used without a router outlet as long asion-tabis a child./tabs-basicDoes this introduce a breaking change?
Developers who are still using
ion-tabswithion-router-outletwill not experience any changes. This feature PR introduces another option to useion-tabs, no changes for the current implementation.Other information
Dev build: 8.2.8-dev.11724363936.1f6132ca