Skip to content
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

fix(speaker-detail-module/navigation): Fixed the back navigation in the header of speakers page #61

Conversation

idobrodushniy
Copy link
Contributor

@idobrodushniy idobrodushniy commented Apr 22, 2023

Fixes #60

Changes:

  • Added a query param to navigate between the session detailed page and the speakers page.
  • It as well covers the use case with opening a session page from the speakers tab, then going to speaker detailed page and then back.

Manual testing guide:

  • Open app/tabs/schedule
  • Click on one of the sessions that has speakers -> e.g. /app/tabs/schedule/session/26
  • Click on the speaker's page -> /app/tabs/schedule/speakers/speaker-details/98
  • Now click the back navigation button in the header
  • You should be brought back to the /app/tabs/schedule/session/26. (before it would be /app/tabs/speakers)
  • Open /app/tabs/speakers now.
  • Open one of the sessions for one of the speakers -> e.g. /app/tabs/speakers/session/75.
  • Now click on the speaker -> you will be redirected to /app/tabs/schedule/speakers/speaker-details/10.
  • Press back -> you should be brought back to /app/tabs/speakers/session/75. Before it would be /app/tabs/schedule/speakers/.

Context

⚠️ I am absolutely terrible in front-end and never worked neither with ionic nor with Angular on top of it, sorry in advance if this solution seems too dummy 😄
At first I thought this is the only one solution, meanwhile later on I checked more in Angular + Ionic documentation.
It seems that issue happens because Angular requires nesting routes in order to preserve the history of navigation stack, otherwise it redirects to a root route that doesn't match a prefix of the current component hence causes reset of a navigation stack history.

The second option to solve it is by creating a separate route nested inside the schedule prefix. (similar to what is done with sessions in

)
Meanwhile imo it is a bad practice leading towards more bugs, because then it introduces circular dependencies between different url prefixes.
Probably there is a more elegant way we could do it 🙂

You can find a diff for main for the second option below ⬇️

diff --git a/src/app/pages/session-detail/session-detail.html b/src/app/pages/session-detail/session-detail.html
index aab7c842..634a9469 100644
--- a/src/app/pages/session-detail/session-detail.html
+++ b/src/app/pages/session-detail/session-detail.html
@@ -30,7 +30,7 @@
           <ion-col size="12" size-md="6" *ngFor="let speaker of session.speakers">
             <ion-card class="speaker-card">
               <ion-card-header>
-                <ion-item detail="false" lines="none" class="speaker-item" routerLink="/app/tabs/speakers/speaker-details/{{speaker.id}}">
+                <ion-item detail="false" lines="none" class="speaker-item" routerLink="/app/tabs/schedule/speakers/speaker-details/{{speaker.id}}">
                   <ion-avatar slot="start">
                     <img [src]="speaker.profilePic" [alt]="speaker.name + ' profile picture'">
                   </ion-avatar>
diff --git a/src/app/pages/tabs-page/tabs-page-routing.module.ts b/src/app/pages/tabs-page/tabs-page-routing.module.ts
index db1b896f..fc1ab283 100644
--- a/src/app/pages/tabs-page/tabs-page-routing.module.ts
+++ b/src/app/pages/tabs-page/tabs-page-routing.module.ts
@@ -2,8 +2,6 @@ import { NgModule } from '@angular/core';
 import { RouterModule, Routes } from '@angular/router';
 import { TabsPage } from './tabs-page';
 import { SchedulePage } from '../schedule/schedule';
-import { ScheduleListPageModule } from '../schedule-list/schedule-list.module';
-
 
 const routes: Routes = [
   {
@@ -20,7 +18,11 @@ const routes: Routes = [
           {
             path: 'session/:sessionId',
             loadChildren: () => import('../session-detail/session-detail.module').then(m => m.SessionDetailModule)
-          }
+          },
+          {
+            path: 'speakers/speaker-details/:speakerId',
+            loadChildren: () => import('../speaker-detail/speaker-detail.module').then(m => m.SpeakerDetailModule)
+          },
         ]
       },
       {
@@ -46,7 +48,7 @@ const routes: Routes = [
           {
             path: 'speaker-details/:speakerId',
             loadChildren: () => import('../speaker-detail/speaker-detail.module').then(m => m.SpeakerDetailModule)
-          }
+          },
         ]
       },
       {

…he header of speakers page -> step back to previous route by default or to the default speakers page if no previous route exists.
@ewdurbin
Copy link
Member

🙈 linting is my fault

@ewdurbin ewdurbin merged commit 8cb1b2a into psf:main Apr 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Navigation from a speaker detailed screen brings to a speakers list screen instead of previous route
2 participants