@christopher-adolphe
Posted
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
andPricing
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 byid
from theImageService
.
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 usingngTemplateOutlet
andngTemplateOutletContext
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:
I hope this helps.
Keep it up.
Marked as helpful
@ohermans1
Posted
@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
@christopher-adolphe
Posted
@ohermans1
Hi mate,
I'm happy to help and glad to see that it was helpful.
There's no urgency 😜 Enjoy your holidays.
Cheers