Feat/header - #3
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new responsive site header with desktop navigation and a mobile menu overlay, and wires up fragment/anchor scrolling to support in-page navigation.
Changes:
- Implemented a feature-rich
Headercomponent (mobile menu, focus handling, scroll locking, navigation/social links, language toggle UI). - Enabled router anchor scrolling via
withInMemoryScrolling({ anchorScrolling: 'enabled' }). - Added new logo/navigation/social/decoration SVG assets used by the header UI, while removing placeholder/previous page markup.
Reviewed changes
Copilot reviewed 7 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/app/pages/home/home.html | Removed the previous homepage hero text (now empty). |
| src/app/layout/header/header.ts | Added header behavior (menu open/close, scroll lock, focus management, listeners). |
| src/app/layout/header/header.spec.ts | Removed header unit tests. |
| src/app/layout/header/header.model.ts | Added typed models for navigation + social link data. |
| src/app/layout/header/header.html | Added full header + mobile navigation menu markup. |
| src/app/layout/header/header.css | Header stylesheet referenced in PR metadata (no diff shown). |
| src/app/layout/footer/footer.html | Removed placeholder footer content (now empty). |
| src/app/app.config.ts | Enabled in-memory anchor scrolling for fragment navigation. |
| public/logo.svg | Added site logo asset. |
| public/assets/icons/social/linkedin.svg | Added LinkedIn social icon asset. |
| public/assets/icons/social/github.svg | Added GitHub social icon asset. |
| public/assets/icons/social/email.svg | Added email social icon asset. |
| public/assets/icons/navigation/menu-open.svg | Added mobile “open menu” icon. |
| public/assets/icons/navigation/menu-close.svg | Added mobile “close menu” icon. |
| public/assets/icons/navigation/menu-active.svg | Added “active/open” menu icon (contains embedded raster data). |
| public/assets/icons/navigation/close-active.svg | Added “active/pressed” close icon (contains embedded raster data). |
| public/assets/decorations/navigation/nav-hover-skills.svg | Added desktop nav hover decoration for “Skills”. |
| public/assets/decorations/navigation/nav-hover-projects.svg | Added desktop nav hover decoration for “Projects”. |
| public/assets/decorations/navigation/nav-hover-contact.svg | Added desktop nav hover decoration for “Contact”. |
| public/assets/decorations/navigation/nav-hover-about.svg | Added desktop nav hover decoration for “About me”. |
| public/assets/decorations/language-hover-en.svg | Added language toggle hover decoration (EN). |
| public/assets/decorations/language-hover-de.svg | Added language toggle hover decoration (DE). |
Suppressed comments (1)
src/app/pages/home/home.html:1
- The Home page template is now empty, so navigating to the root route ("/") will render only the global header/footer and no page content. If this content was not moved elsewhere, this is a functional regression.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| protected toggleLanguage(): void { | ||
| this.activeLanguage.update((language) => (language === 'en' ? 'de' : 'en')); | ||
| } | ||
|
|
||
| protected openMobileMenu(): void { | ||
| if (this.isMobileMenuOpen()) { | ||
| return; | ||
| } | ||
|
|
||
| this.isMobileMenuOpen.set(true); | ||
| this.lockBodyScroll(); | ||
|
|
||
| afterNextRender( | ||
| () => { | ||
| if (this.isMobileMenuOpen()) { | ||
| this.closeMenuButton().nativeElement.focus({ | ||
| preventScroll: true, | ||
| }); | ||
| } | ||
| }, | ||
| { | ||
| injector: this.injector, | ||
| }, | ||
| ); | ||
| } | ||
|
|
||
| protected closeMobileMenu(restoreFocus = true): void { | ||
| if (!this.isMobileMenuOpen()) { | ||
| return; | ||
| } | ||
|
|
||
| if (restoreFocus) { | ||
| this.openMenuButton().nativeElement.focus({ | ||
| preventScroll: true, | ||
| }); | ||
| } | ||
|
|
||
| this.isMobileMenuOpen.set(false); | ||
| this.restoreBodyScroll(); | ||
| } | ||
|
|
||
| @HostListener('document:keydown.escape') | ||
| protected onEscape(): void { | ||
| if (!this.isMobileMenuOpen()) { | ||
| return; | ||
| } | ||
|
|
||
| this.closeMobileMenu(); | ||
| } | ||
|
|
||
| @HostListener('window:resize') | ||
| protected onViewportResize(): void { | ||
| const isDesktop = this.document.defaultView?.matchMedia(DESKTOP_MEDIA_QUERY).matches ?? false; | ||
|
|
||
| if (isDesktop) { | ||
| this.closeMobileMenu(false); | ||
| } | ||
| } | ||
|
|
||
| ngOnDestroy(): void { | ||
| this.restoreBodyScroll(); |
| @HostListener('window:resize') | ||
| protected onViewportResize(): void { | ||
| const isDesktop = this.document.defaultView?.matchMedia(DESKTOP_MEDIA_QUERY).matches ?? false; | ||
|
|
||
| if (isDesktop) { | ||
| this.closeMobileMenu(false); | ||
| } | ||
| } |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c1b12dd71
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| routerLink="/" | ||
| [fragment]="item.fragment" |
There was a problem hiding this comment.
Every desktop and mobile navigation item routes to fragments such as /#about, but a repository-wide search finds no matching about, skills, projects, or contact element IDs, and the home template is now empty. Consequently, selecting any primary navigation item only changes the URL and cannot scroll to or display the requested section.
Useful? React with 👍 / 👎.
| protected toggleLanguage(): void { | ||
| this.activeLanguage.update((language) => (language === 'en' ? 'de' : 'en')); | ||
| } |
There was a problem hiding this comment.
When a user selects DE, this handler only changes the switch's colors and knob position; all navigation labels, accessible labels, and other content remain hard-coded in English, with no translation service or language-dependent data elsewhere in the repository. The control therefore announces that the language was switched without changing the interface language.
Useful? React with 👍 / 👎.
| [attr.aria-hidden]="!isMobileMenuOpen()" | ||
| [attr.inert]="isMobileMenuOpen() ? null : ''" |
There was a problem hiding this comment.
When the mobile menu is open, inert is removed only from the menu while the underlying mobile header remains focusable. From the initially focused close button, keyboard users can Shift+Tab through the menu logo and onto the obscured open-menu button, so focus escapes behind the overlay; the background should be made inert or focus should be trapped until the menu closes.
Useful? React with 👍 / 👎.
No description provided.