@christopher-adolphe
Posted
Hi @MoodyJW 👋
You did a great job on this challenge. 👍 The component composition is well done. 👌
The only thing I have noticed is that the <body> tag is inside your root AppComponent and in the rendered index.html page you now have a <body> tag nested in another <body> tag which is unfortunately not valid.
I understand you did this to implement the theme switching feature. Fortunately, you could resolve this with the help of Renderer2 from @angular/core module. By injecting this service and the DOCUMENT into your AppComponent class, you will be able to add/remove the theme class on the <body> element the "Angular way" without having it inside the root AppComponent template file. Your AppComponent should look something like this after doing this change:
import { Component, OnInit, Inject, Renderer2 } from '@angular/core';
import {DOCUMENT} from "@angular/common";
/**
* Your other imports, component decorator etc
*/
export class AppComponent implements OnInit {
theme = window.matchMedia('(prefers-color-scheme: light)').matches
? 'light-theme'
: 'dark-theme';
user!: User;
constructor(
@Inject(DOCUMENT) private document: Document,
private renderer: Renderer2,
private usersService: UsersService
) {}
ngOnInit() {
this.renderer.addClass(this.document.body, this.theme);
this.usersService
.getUser('octocat')
.pipe(first())
.subscribe((user) => {
this.user = user;
});
}
toggleTheme() {
const bodyElem = this.document.body;
const isLightTheme = bodyElem.classList.contains('light-theme');
if (!isLightTheme) {
this.renderer.removeClass(bodyElem, 'dark-theme');
this.renderer.addClass(bodyElem, 'light-theme');
} else {
this.renderer.removeClass(bodyElem, 'light-theme');
this.renderer.addClass(bodyElem, 'dark-theme');
}
}
userReturned(user: any) {
this.user = user;
}
}
However, with this change, the currentTheme argument passed to the toggleTheme() method would no more be required and therefore, the ThemeToggleComponent would no more need to emit a custom event.
Even better, you could move the toggleTheme() method from the root AppComponent to the ThemeToggleComponent since it is the one responsible of this feature. The root AppComponent would thus be only responsible of initialising the user property.
I hope this helps.
Keep it up.
Marked as helpful
@MoodyJW
Posted
@christopher-adolphe this helps a ton! Thank you so much for taking the time to leave such a detailed explanation. Your approach definitely makes more sense and solves that extra body tag problem too.
@christopher-adolphe
Posted
Hi @MoodyJW,
I'm happy to help and glad to see this was helpful to you. 🙂
I don't see many solutions built with Angular here on Frontend Mentor, so whenever I see one, I always have a look at the code and see how others approach the challenges.
See you on the next one.
Best regards.
