PhotoSnap - Created with Angular - Includes extra functionality

Community feedback

• 

  P

  Christopher Adolphe•600

  @christopher-adolphePosted over 2 years ago

  Hi @ohermans1,

  You did a good job for this challenge. 👍 I like your responsive approach for the comparison table on the Pricing page. 👌 I've seen very few solutions implemented with Angular over here, so I had a look at your github repository and here are a few things I have noticed and you want to check in order to improve your solution.

  While this is a fairly medium sized Angular project, it is important to organise your components' imports. I think that at the moment, the app.module.ts is quite bloated; all components are loaded while they are not all immediately used. You could leverage on feature modules to streamline this and at the same time you could also benefit from lazy-loading so that only the necessary modules are loaded when required. Below is how I would suggest refactoring this.

  • Create feature modules for Home, Stories, Features and Pricing with each having its routing configuration. For example:

  home.module.ts

  import { NgModule } from '@angular/core';
  
  import { SharedModule } from '../shared/shared.module';
  import { HomeRoutingModule } from './home-routing.module';
  import { HomeComponent } from './home.component';
  import { ContentComponent } from './content/content.component';
  
  @NgModule({
    declarations: [
      HomeComponent,
      ContentComponent
    ],
    imports: [
      SharedModule,  // <= Importing shared components via this shared module
      HomeRoutingModule
    ]
  })
  export class HomeModule { }
  

  home-routing.module.ts

  import { NgModule } from '@angular/core';
  import { RouterModule, Routes } from '@angular/router';
  
  import { HomeComponent } from './home.component';
  
  const routes: Routes = [
    {
      path: '',
      component: HomeComponent
    }
  ];
  
  @NgModule({
    imports: [RouterModule.forChild(routes)],
    exports: [RouterModule]
  })
  export class HomeRoutingModule { }
  

  • Lazy-load each module by configuring the app-routing.module.ts.

  app-routing.module.ts

  import { NgModule } from '@angular/core';
  import { RouterModule, Routes } from '@angular/router';
  
  const  routes: Routes = [
    {
      path: 'home',
      loadChildren: () => import('./home/home.module').then(m  =>  m.HomeModule)
    },
    {
      path: 'stories',
      loadChildren: () => import('./stories/stories.module').then(m  => m.StoriesModule)
    },
    {
      path: 'features',
      loadChildren: () => import('./features/features.module').then(m  => m.FeaturesModule)
    },
    {
      path: 'pricing',
      loadChildren: () => import('./pricing/pricing.module').then(m  =>  m.PricingModule)
    },
    {
      path: '',
      redirectTo: '',
      pathMatch: 'full'
    }
  ];
  
  @NgModule({
    imports: [RouterModule.forRoot(routes)],
    exports: [RouterModule]
  })
  export class AppRoutingModule { }
  

  • Move all the other shared components to a shared.module.ts that can be imported by the feature modules.

  shared.module.ts

  import { NgModule } from '@angular/core';
  import { CommonModule } from '@angular/common';
  
  import { FeaturesGroupComponent } from './features-group/features-group.component';
  import { GalleryComponent } from './gallery/gallery.component';
  import { ImageCardComponent } from './image-card/image-card.component';
  import { TopBannerComponent } from './UI/top-banner/top-banner.component';
  import { BottomBannerComponent } from './UI/bottom-banner/bottom-banner.component';
  
  @NgModule({
    declarations: [
      FeaturesGroupComponent,
      GalleryComponent,
      ImageCardComponent,
      TopBannerComponent,
      BottomBannerComponent
    ],
    imports: [
      CommonModule
    ],
    exports: [
      CommonModule,
      FeaturesGroupComponent,
      GalleryComponent,
      ImageCardComponent,
      TopBannerComponent,
      BottomBannerComponent
    ]
  })
  export class SharedModule { }
  

  • Then import only components that are required on initial load of the application in app.module.ts

  app.module.ts

  import { NgModule } from '@angular/core';
  import { BrowserModule } from '@angular/platform-browser';
  
  import { AppRoutingModule } from './app-routing.module';
  import { AppComponent } from './app.component';
  import { HeaderComponent } from './shared/UI/header/header.component';
  import { FooterComponent } from './shared/UI/footer/footer.component';
  
  @NgModule({
    declarations: [
      AppComponent,
      HeaderComponent,
      FooterComponent
    ],
    imports: [
      BrowserModule,
      AppRoutingModule
    ],
    providers: [],
    bootstrap: [AppComponent],
  })
  export class AppModule {}
  

  • I have noticed that you have reset your routing configurations to achieve the inner routes for the individual stories. Maybe you could consider using a child route where you pass an id and then get the story by id from the ImageService.

  stories-routing.module.ts

  import { NgModule } from  '@angular/core';
  import { RouterModule, Routes } from  '@angular/router';
  
  import { StoriesComponent } from  './stories.component';
  import { IndividualStoryComponent } from  './individual-story/individual-story.component';
  
  const routes: Routes = [
    {
      path: '',
      component: StoriesComponent,
      children: [
        {
          path: ':id',
          component: IndividualStoryComponent
        }
      ]
    }
  ];
  
  @NgModule({
    imports: [RouterModule.forChild(routes)],
    exports: [RouterModule]
  })
  export class StoriesRoutingModule { }
  

  • For the features table on the Pricing page, you may consider using ngTemplateOutlet and ngTemplateOutletContext to generate the rows of the table with the help of an *ngFor directive.

  ...
  <tbody class="table__body">
    <ng-container
      *ngFor="let feature of features"
      [ngTemplateOutlet]="featureItemTpl"
      [ngTemplateOutletContext]="{ featureItem: feature }"></ng-container>
  </tbody>
  ...
  
  <ng-template #featureItemTpl let-item="featureItem">
    <tr>
      <th>
        <h4>{{ item.title }}</h4>
      </th>
      <th>
        <img [src]="item.imgPath" alt="{{ item.title }}" />
      </th>
    </tr>
  </ng-template>
  

  If you wish to read more about feature/shared modules and lazy-loading, check the links below:

  • Feature modules
  • Shared modules
  • Lazy-loading modules

  I hope this helps.

  Keep it up.

  Marked as helpful

  1

  ---

  

  Ollie•580

  @ohermans1Posted over 2 years ago

  @christopher-adolphe

  Hey mate,

  Wow! Thank you so much for the awesome feedback.

  I'm heading away on holiday, but will read through this properly as soon as I can, I looks like super helpful interesting stuff.

  I'm looking forward to seeing future solutions from you!

  Ollie

  0

  

  P

  Christopher Adolphe•600

  @christopher-adolphePosted over 2 years ago

  @ohermans1

  Hi mate,

  I'm happy to help and glad to see that it was helpful.

  There's no urgency 😜 Enjoy your holidays.

  Cheers

  0
• 

  Rachael•610

  @RMK-creativePosted over 2 years ago

  Hi Ollie, awesome job on this project - it looks great on different screen sizes I tested with dev tools. I especially love that you added the extra pages, really nice touch!

  Just one thing I came across - when I click on the image card the story page will load, but when I click on the "read more ->" section of the same card, it skips past the story page and loads the home page again. When I click back on my browser, I then see the story page. Maybe just checkout the functionality of that read more link :)

  Marked as helpful

  1

  ---

  

  Ollie•580

  @ohermans1Posted over 2 years ago

  @RMK-creative

  Hey Rachael

  Thanks so much for your awesome feedback and for picking up the bug with the 'read more' button.

  Turns out I had left a href="#" on the 'link button' for those image card components, hence the re-routing back to the home page. It should all be up and running as expected again now.

  Thanks again, I will be sure to check out some of your projects.

  Regards

  Ollie

  0