ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
51.09k stars 13.51k forks source link

bug: angular, tabs does not support root path (without /tabs) #16949

Open YDrogen opened 5 years ago

YDrogen commented 5 years ago

Bug Report

Ionic version: [x] 4.x

Current behavior: Using the ion-tabs component, clicking on ion-tab-buttons redirect to wrong URL when trying to remove /tabs from tabs.router.module.ts.

Expected behavior: Clicking on one ion-tab-button should redirect to correct URL.

Steps to reproduce:

  1. Start a new Ionic Angular project (ionic start tabs-issue tabs --type=angular)
  2. Remove /tabs path from tabs.router.module.ts (see related code)
  3. Start the app
  4. Try navigating from one tab to another

Related code:

const routes: Routes = [
  {
    path: '',
    component: TabsPage,
    children: [
      {
        path: 'tab1',
        children: [
          {
            path: '',
            loadChildren: '../tab1/tab1.module#Tab1PageModule'
          }
        ]
      },
      {
        path: 'tab2',
        children: [
          {
            path: '',
            loadChildren: '../tab2/tab2.module#Tab2PageModule'
          }
        ]
      },
      {
        path: 'tab3',
        children: [
          {
            path: '',
            loadChildren: '../tab3/tab3.module#Tab3PageModule'
          }
        ]
      },
      {
        path: '',
        redirectTo: '/tab1',
        pathMatch: 'full'
      }
    ]
  }
];

Other information: To make it simple - While on /tab1, clicking on the second tab leads me to /tab1/tab2 instead of /tab2

Ionic info:

Ionic:

   ionic (Ionic CLI)             : 4.3.1 (C:\Users\YDrogen\AppData\Roaming\npm\node_modules\ionic)
   Ionic Framework               : @ionic/angular 4.0.0-rc.0
   @angular-devkit/build-angular : 0.11.4
   @angular-devkit/schematics    : 7.1.4
   @angular/cli                  : 7.1.4
   @ionic/angular-toolkit        : 1.2.2

System:

   NodeJS : v8.9.1 (C:\Program Files\nodejs\node.exe)
   npm    : 5.6.0
   OS     : Windows 10
paulstelzer commented 5 years ago

Thanks for your issue! Could you please add the code of your TabsPage. I am sure there must be the issue.

YDrogen commented 5 years ago

I'm using the default tabs template, I forgot to specify

Using the API documentation and the release notes (4.0.0-beta.18) on how to use tabs routing it should work. But I guess ion-tab-button is built to use with /tabs, which means when it's not there it uses the first path param as if it was /tabs eg. /tab1/tab2 while Ionic think it's /tabs/tab2

tabs.page.ts:

import { Component } from '@angular/core';

@Component({
  selector: 'app-tabs',
  templateUrl: 'tabs.page.html',
  styleUrls: ['tabs.page.scss']
})
export class TabsPage {}

tabs.page.html:

<ion-tabs>

  <ion-tab-bar slot="bottom">
    <ion-tab-button tab="tab1">
      <ion-icon name="flash"></ion-icon>
      <ion-label>Tab One</ion-label>
    </ion-tab-button>

    <ion-tab-button tab="tab2">
      <ion-icon name="apps"></ion-icon>
      <ion-label>Tab Two</ion-label>
    </ion-tab-button>

    <ion-tab-button tab="tab3">
      <ion-icon name="send"></ion-icon>
      <ion-label>Tab Three</ion-label>
    </ion-tab-button>
  </ion-tab-bar>

</ion-tabs>
paulstelzer commented 5 years ago

Tabs needs a tabs prefix -> https://github.com/ionic-team/ionic/blob/master/angular/src/directives/navigation/ion-tabs.ts#L62

This comes from the intern ion-router-outlet -> https://github.com/ionic-team/ionic/blob/master/angular/src/directives/navigation/ion-router-outlet.ts#L62

But you removed it, so he uses "tab1" which is now his prefix.

@manucorporat Could you say if this is needed or could you add a check if there is no tabsPrefix

nickmeulen commented 5 years ago

The need for a tabs prefix is highly inconvenient for good url patterns when building a pwa, especially when the tabs are at the root of the application. Works fine when using href instead of tab but the app it self reloads. Would be nice to be able to disable tabsPrefix

roxteddy commented 5 years ago

The url prefix should be the path of the component where we put the tabs. In my case, I can't place tabs at the root of the app. And I can't neither place the word "tabs" in the url because of the nested router bug (#17992), I need to share tabs router with some others no-tabbed components.

bijanmmarkes commented 5 years ago

This is true, what's the point of clean URLs if we have to have a /tabs prefix. This makes for a very ugly URL scheme, and really is pointless as tabs should be invisible, not part of the URL.

Please, any information on this as this issue is a big one. I think forcing a /tabs prefix before each path is very dirty.

Any updates? 👍

tlaverdure commented 5 years ago

Would like to also remove the tabs prefix. I'm building for web and mobile and I'd like to keep the urls for content within the app the same to ensure that bookmarking/sharing is consistent across the board.

joaquinjsb commented 5 years ago

Hi, any ideas on when it'll be solved ? I think I found an workaround for this, I'm using the following scheme: entity/:id, I wanted to have on the root of that path the information of that entity, and on /entity/:id/relationship another tab with that info, I achieved this doing the following:

example using schedule entity: on app-routing.module

{
                path: 'schedule/:id',
                loadChildren: () => import('./module/schedule-tab/schedule-tab.module').then(m => m.ScheduleTabPageModule)
 }

on ScheduleTabPageModule

const routes: Routes = [
    {
        path: '',
        component: ScheduleTabPage,
        children: [
            {
                path: '',
                loadChildren: () => import('../../page/schedule-detail/schedule-detail.module').then(m => m.ScheduleDetailPageModule)
            },
            {
                path: 'executions',
                loadChildren: () => import('../../page/execution/execution.module').then(m => m.ExecutionPageModule)
            }
        ]
    }
];

giving the expected routing, the problem is that the default route on the tab never looks like active, so that's the only problem, I use on the tab-button "/" as url, if I leave it as "", then it doesn't work.

another problem is that I would need to fetch that :Id, no idea on how to gather it

ASHFAQPATWARI commented 5 years ago

@liamdebeasi is there any update on this issue. this is a real deal breaker for building pwa

edlo commented 5 years ago

@liamdebeasi I'm also looking to see if there has been any update to this.

I'd really like to get around the fact that ion-tabs require a prefix. I don't want to have to customize my own tab bar, but I also don't want to have /tabs in my url.

ghost commented 4 years ago

Glad I found a thread of people with the same issue as me. This was so hard for me to try and solve haha

quincarter commented 4 years ago

bumping this. i am also building a pwa/android/ios app and the /tabs/*** in the url is so annoying. There has to be some kind of variable we could add that could specify whether or not the tabsPrefix is needed. I really liked that idea, and it wouldn't be too hard to add as an @Input or props on the component to pass in or something.

I can modify this line in the source in my node_modules @ionic/angular/esm2015/directives/navigation/ion-tabs.js:47 (here in the github: https://github.com/ionic-team/ionic/blob/master/angular/src/directives/navigation/ion-tabs.ts#L92)

to be this:

const tabRootUrl = !this.outlet.tabsPrefix ? `${tab}` : `${this.outlet.tabsPrefix}/${tab}`;

And it works on my local environment. With some modification to my tabs-routing.module.ts. And it doesn't work if i have a redirect in there to anything but the ''. The home page shows up blank right now until the first tab is selected, but i think it is close. I have been pulling my hair out about this for days. Need another set of fresh eyes on this, but this should help get someone started on an ending solution i think.

here's my tabs-routing.module.ts

import { NgModule } from '@angular/core';
import { RouterModule, Routes } from '@angular/router';
import { TabsPage } from './tabs.page';

const routes: Routes = [
  {
    path: '',
    component: TabsPage,
    children: [
      {
        path: 'home',
        children: [
          {
            path: '',
            loadChildren: () => import('../tab1/tab1.module').then(m => m.Tab1PageModule)
          }
        ]
      },
      {
        path: 'calculate',
        children: [
          {
            path: '',
            loadChildren: () => import('../tab2/tab2.module').then(m => m.Tab2PageModule)
          }
        ]
      },
      {
        path: 'my-stuff',
        children: [
          {
            path: '',
            loadChildren: () => import('../tab3/tab3.module').then(m => m.Tab3PageModule)
          }
        ]
      },
      {
        path: '',
        redirectTo: '',
        pathMatch: 'full'
      }
    ]
  }
];

@NgModule({
  imports: [RouterModule.forChild(routes)],
  exports: [RouterModule]
})
export class TabsPageRoutingModule {}

This is something i am noticing if i land on any of those routes by default -- i.e. localhost:8100/home or localhost:8100/calculate -- if i land on the root then navigate it is fine, but then if i start on any of those, this happens. (in this instance i started on /home) Screenshot from 2020-03-11 22-42-38

koshyviv commented 4 years ago

Didn't think there would be open thread for this, but please do check if its possible to remove the prefix. Thanks!

ogix commented 4 years ago

https://github.com/ionic-team/ionic/blob/53fc8e37c89cea793d0e00246d52805166730108/angular/src/directives/navigation/ion-router-outlet.ts#L77 I found this line that wrongly reads tabsPrefix if your route consists of only one segment. So if your tab route is just /home then getUrl returns /home and IonRouterOutlet saves it as a tabs prefix which is wrong and breaks other things. I tried changing this line to: this.tabsPrefix = tabs === 'true' ? '/' : undefined; and it seems to be working. Ionic team, please make a way to override this. The first idea that comes to my mind is providing a new InjectionToken that IonRouterOutlet would optionally inject into its constructor and then and we would be able to set it by registering some value. What do you think?

jonathan-chin commented 4 years ago

https://github.com/ionic-team/ionic/blob/53fc8e37c89cea793d0e00246d52805166730108/angular/src/directives/navigation/ion-router-outlet.ts#L77

I found this line that wrongly reads tabsPrefix if your route consists of only one segment. So if your tab route is just /home then getUrl returns /home and IonRouterOutlet saves it as a tabs prefix which is wrong and breaks other things. I tried changing this line to: this.tabsPrefix = tabs === 'true' ? '/' : undefined; and it seems to be working. Ionic team, please make a way to override this. The first idea that comes to my mind is providing a new InjectionToken that IonRouterOutlet would optionally inject into its constructor and then and we would be able to set it by registering some value. What do you think?

any way we can get this option?

supabitra commented 4 years ago

Hello folks, is there any update on this issue please? It will be really helpful to get a fix. Please let me know if any part of the issue is not clear, I can try to reproduce the issue and share the corresponding code for investigation. Please help. Thanks.

vicmarcal commented 3 years ago

Any news regarding the enforced Prefix? It looks pretty odd to have the “tab” prefix in URLs (PWA) or any workaround/template?

Thanks!