refactor: add app routing and lazy load home feature

[Mubramaj]

Changes in this pull request

• [x] create the main app routing
• [x] lazy load home module
• [x] fix existing tests
• [x] add some tests for home component
• [x] add error display when fetch data request fails and no videos available

I think there is a little flickering the first time we render the page when using SSR.

I think this is due to the first pre-rendering content, then component run in browser, displays the spinner, fetch data and render it again.

[netlify[bot]]

Deploy request for ngx-darija rejected.

Rejected with commit 332f310381df150b223dfb48475243de96021012

https://docs.netlify.com/configure-builds/environment-variables/#sensitive-variable-policy

[ikourfaln]

Hi @Mubramaj

Did you remove ViewEncapsulation.None because of empty SCSS ? which mean if SCSS is empty, the preprocessor of the component style will not start, so there is no need to set the Encapsulation.

if that is the right behavior of Angular, that's a good info to know 😉

[Mubramaj]

Hi @ikourfaln,

No I removed ViewEncapsulation.None to keep default behaviour of Angular which encapsulate each component .scss meaning that for example if you do a style in your foo.component.scss like

a {
   color: red;
}

Then only the links in the foo.component.html will be red. If you set ViewEncapsulation.None then all the a tags in the entire app will be Red. Which can cause a lot of side effects and a headache when debugging the css :) .

I also removed empty scss files just to clean up project.

I hope it make sense, let me know what you think

[ikourfaln]

thank you @Mubramaj for reply

Yes, I already know the behavior of ViewEncapsulation.None, but because we know that we will work only with Tailwind css (already added to the project), there is no need for encapsulation, even Tailwind css community advice that, except if you are forced to work with style file, bcz it helps to avoid some issues, plus there no need to start the preprocessor of the component style (Encapsulation) if it's empty.

that's why I asked you if Angular is smart enough to ignore the Encapsulation if style file is empty.

[Mubramaj]

@ikourfaln my bad, I never worked with Tailwind css.

I will add back encapsulation None. that's why I asked you if Angular is smart enough to ignore the Encapsulation if style file is empty. I don't know the answer to this question

So if I want to set a css class to use in several parts in the app where should I add this class ? (usually I do it in styles.scss but with Tailwind I don't know

Also if I want to define css styles encapsulated for only 1 component, how should I do it with Tailwind ?

[ikourfaln]

@Mubramaj no problem

Tailwind css has almost all you need (layout, spacing, colors, responsive...etc) as utility classes.

So if you have really a specific style need you wanna use it in several parts in the app, you can do it in styles.scss, as usual. (but as I said, usually Tailwind css classes has everything u need)

And if you are enforced to style a component and encapsulated it, you can remove ViewEncapsulation.None.

nothing stops you to do what you want, it's just better to follow best practices and conventions 😉

PS: you can use Tailwind css plugin for VS Code, it really helps

[Mubramaj]

@ikourfaln thanks for the answer.

Ok cool, good to know. I used styles.scss to create a class to set on an element for it to take all spacing between header and footer, will commit soon in this PR

For the css classes of Tailwind, I usually use bootstrap and they have utility classes so I figured Tailwind also had the same thing. I looked at their doc.

[ikourfaln]

You are welcome @Mubramaj Waiting to rise your Tailwaind skills :stuck_out_tongue_winking_eye: , you can use styles.scss

(your use case (spacing), you can use https://tailwindcss.com/docs/padding, https://tailwindcss.com/docs/margin, or https://tailwindcss.com/docs/space)

PS: Tailwind and Bootstrap have different philosophy

[chihab]

Thanks for this awesome PR @Mubramaj!

Data should always be available because the site is statically generated at build time (unless the request fails at build time which should/would also be handled). I am not sure whether we should display the loading spinner when the Client side request is made, because data is already here, currently there should be no update of the DOM unless some data has changed on the Playlist as in the following scenario:

After deploying your branch here: https://604d4d75da06b400080e7097--ngx-darija.netlify.app I removed "Salam" from the descriptions. You can see the update on the screen after the first display with "Salam".

After updating the playlist, we should usually trigger another build to pre-render with lastest data.

[Mubramaj]

Hi @chihab , I think what is deployed in https://604d4d75da06b400080e7097--ngx-darija.netlify.app is another version.

If I understand correctly, spinner not needed because first request already have static data with the playlists.

My idea behind the spinner was the following: When there will be more than 1 view, and routing will be handled inside angular ( meaning after the first page refresh, user navigate to session details view, then comes back to the home page). Then client does request to fetch playlist and if request is slow, there will be a blank page.

I am going to fix the conflicts, let me know if I should remove spinner for now since we only have 1 page

[ikourfaln]

@Mubramaj I think that it will be good if you move shared components from _shared/ui to _shared/components so we can follow the same conventions

[Mubramaj]

@ikourfaln Good catch, will do that later tonight !

[Mubramaj]

Done !

@chihab I removed the spinner since we create static file at build time.

It should be good to go unless there is more feedback ;)

[chihab]

Thanks for the update @Mubramaj!