-
Notifications
You must be signed in to change notification settings - Fork 37
web-next: Replace AppSidebar with responsive Navigation #194
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
0e1470b
89de3f9
cd79607
080f65a
e4047cc
47b34ba
fe09d85
a17a87d
eedf79c
eaee1e8
0603dd3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| import { useLingui } from "~/lib/i18n/macro.d.ts"; | ||
| import { Trans } from "./Trans.tsx"; | ||
|
|
||
| export interface FooterProps { | ||
| class?: string; | ||
| } | ||
|
|
||
| export function Footer(props: FooterProps) { | ||
| const { t } = useLingui(); | ||
|
|
||
| return ( | ||
| <footer | ||
| class={`p-4 text-xs text-muted-foreground ${props.class ?? ""}`} | ||
| > | ||
| <p class="underline mb-2"> | ||
| <a href="/coc">{t`Code of conduct`}</a> | ||
| </p> | ||
| <p> | ||
| <Trans | ||
| message={t`The source code of this website is available on ${"GITHUB_REPOSITORY"} under the ${"AGPL-3.0"} license.`} | ||
| values={{ | ||
| GITHUB_REPOSITORY: () => ( | ||
| <a | ||
| href="https://github.com/hackers-pub/hackerspub" | ||
| target="_blank" | ||
| class="underline" | ||
| > | ||
| {t`GitHub repository`} | ||
| </a> | ||
| ), | ||
| "AGPL-3.0": () => ( | ||
| <a | ||
| href="https://www.gnu.org/licenses/agpl-3.0.en.html" | ||
| target="_blank" | ||
| class="underline" | ||
| > | ||
| AGPL 3.0 | ||
| </a> | ||
| ), | ||
| }} | ||
|
Comment on lines
+19
to
+40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Find all .po files and search for the message
fd -e po . web-next/src/locales --exec grep -l "The source code of this website is available on" {} \;Repository: hackers-pub/hackerspub Length of output: 260 🏁 Script executed: #!/bin/bash
# Get details of the message in locale files
rg -A 3 "The source code of this website is available on" web-next/src/localesRepository: hackers-pub/hackerspub Length of output: 1898 Align The locale entries use ✅ Correct placeholder names values={{
- GITHUB_REPOSITORY: () => (
+ "{0}": () => (
<a
href="https://github.com/hackers-pub/hackerspub"
target="_blank"
class="underline"
>
{t`GitHub repository`}
</a>
),
- "AGPL-3.0": () => (
+ "{1}": () => (
<a
href="https://www.gnu.org/licenses/agpl-3.0.en.html"
target="_blank"
class="underline"
>
AGPL 3.0
</a>
),
}}🧰 Tools🪛 Biome (2.1.2)[error] 25-25: Avoid using target="_blank" without rel="noopener" or rel="noreferrer". Opening external links in new tabs without rel="noopener" is a security risk. See the explanation for more details. (lint/security/noBlankTarget) [error] 34-34: Avoid using target="_blank" without rel="noopener" or rel="noreferrer". Opening external links in new tabs without rel="noopener" is a security risk. See the explanation for more details. (lint/security/noBlankTarget) 🤖 Prompt for AI Agents |
||
| /> | ||
| </p> | ||
| </footer> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,162 @@ | ||
| import { A, useLocation } from "@solidjs/router"; | ||
| import { graphql } from "relay-runtime"; | ||
| import { Show } from "solid-js"; | ||
| import { createFragment } from "solid-relay"; | ||
| import { useNoteCompose } from "~/contexts/NoteComposeContext.tsx"; | ||
| import { useLingui } from "~/lib/i18n/macro.d.ts"; | ||
| import IconBell from "~icons/lucide/bell"; | ||
| import IconList from "~icons/lucide/list"; | ||
| import IconSearch from "~icons/lucide/search"; | ||
| import IconSquarePen from "~icons/lucide/square-pen"; | ||
| import type { Navigation_signedAccount$key } from "./__generated__/Navigation_signedAccount.graphql.ts"; | ||
| import { Footer } from "./Footer.tsx"; | ||
| import { Avatar, AvatarFallback, AvatarImage } from "./ui/avatar.tsx"; | ||
|
|
||
| export interface NavigationProps { | ||
| $signedAccount: Navigation_signedAccount$key; | ||
| } | ||
|
|
||
| export function Navigation(props: NavigationProps) { | ||
| const { t } = useLingui(); | ||
| const { open: openNoteCompose } = useNoteCompose(); | ||
| const location = useLocation(); | ||
|
|
||
| const signedAccount = createFragment( | ||
| graphql` | ||
| fragment Navigation_signedAccount on Account { | ||
| username | ||
| avatarUrl | ||
| } | ||
| `, | ||
| () => props.$signedAccount, | ||
| ); | ||
|
|
||
| const isActive = (path: string) => { | ||
| return location.pathname === path || | ||
| location.pathname.startsWith(path + "/"); | ||
| }; | ||
|
|
||
| return ( | ||
| <Show when={signedAccount()} keyed> | ||
| {(account) => ( | ||
| <nav | ||
| class="fixed sm:flex sm:flex-col z-50 border-border bg-background/95 bottom-0 left-0 right-0 border-t sm:sticky sm:top-0 sm:h-screen sm:w-16 sm:shrink-0 sm:border-t-0 sm:border-r lg:w-50 sm:py-3" | ||
| role="navigation" | ||
| aria-label={t`Main navigation`} | ||
| > | ||
| <A | ||
| href="/" | ||
| class="hidden sm:flex items-center justify-center p-3 lg:px-4" | ||
| aria-label={t`Home`} | ||
| > | ||
| <img | ||
| src="/pubnyan-normal-border.svg" | ||
| alt="Hackers' Pub" | ||
| class="size-9 lg:hidden" | ||
| /> | ||
| <picture class="hidden lg:block"> | ||
| <source | ||
| srcset="/logo-dark.svg" | ||
| media="(prefers-color-scheme: dark)" | ||
| /> | ||
| <img | ||
| src="/logo-light.svg" | ||
| alt="Hackers' Pub" | ||
| class="h-10" | ||
| /> | ||
| </picture> | ||
| </A> | ||
| <ul class="flex h-13 items-center justify-around px-2 sm:h-auto sm:flex-col sm:justify-start sm:gap-1 sm:px-0 sm:py-2 lg:pl-6"> | ||
| <li class="flex-1 sm:flex-none sm:w-full"> | ||
| <A | ||
| href="/feed" | ||
| class="flex items-center justify-center py-3 text-muted-foreground transition-colors sm:p-3 lg:justify-start lg:gap-3 lg:rounded-full lg:hover:bg-accent" | ||
| classList={{ | ||
| "text-foreground": isActive("/feed"), | ||
| }} | ||
| aria-label={t`Feed`} | ||
| aria-current={isActive("/feed") ? "page" : undefined} | ||
| > | ||
| <IconList class="size-6 shrink-0" aria-hidden="true" /> | ||
| <span class="hidden lg:inline">{t`Feed`}</span> | ||
| </A> | ||
| </li> | ||
| <li class="flex-1 sm:flex-none sm:w-full"> | ||
| <A | ||
| href="/notifications" | ||
| class="flex items-center justify-center py-3 text-muted-foreground transition-colors sm:p-3 lg:justify-start lg:gap-3 lg:rounded-full lg:hover:bg-accent" | ||
| classList={{ | ||
| "text-foreground": isActive("/notifications"), | ||
| }} | ||
| aria-label={t`Notifications`} | ||
| aria-current={isActive("/notifications") ? "page" : undefined} | ||
| > | ||
| <IconBell class="size-6 shrink-0" aria-hidden="true" /> | ||
| <span class="hidden lg:inline">{t`Notifications`}</span> | ||
| </A> | ||
| </li> | ||
| <li class="flex-1 sm:hidden"> | ||
| <button | ||
| type="button" | ||
| onClick={openNoteCompose} | ||
| class="flex w-full items-center justify-center py-3 text-muted-foreground transition-colors sm:p-3" | ||
| aria-label={t`Create Note`} | ||
| > | ||
| <IconSquarePen class="size-6 shrink-0" aria-hidden="true" /> | ||
| </button> | ||
| </li> | ||
| <li class="flex-1 sm:flex-none sm:w-full"> | ||
| <A | ||
| href="/search" | ||
| class="flex items-center justify-center py-3 text-muted-foreground transition-colors sm:p-3 lg:justify-start lg:gap-3 lg:rounded-full lg:hover:bg-accent" | ||
| classList={{ | ||
| "text-foreground": isActive("/search"), | ||
| }} | ||
| aria-label={t`Search`} | ||
| aria-current={isActive("/search") ? "page" : undefined} | ||
| > | ||
| <IconSearch class="size-6 shrink-0" aria-hidden="true" /> | ||
| <span class="hidden lg:inline">{t`Search`}</span> | ||
| </A> | ||
| </li> | ||
| <li class="flex-1 sm:flex-none sm:w-full"> | ||
| <A | ||
| href={`/@${account.username}`} | ||
| class="flex items-center justify-center py-3 sm:p-3 lg:justify-start lg:gap-3 lg:rounded-full lg:hover:bg-accent" | ||
| aria-label={t`Profile`} | ||
| aria-current={location.pathname === `/@${account.username}` | ||
| ? "page" | ||
| : undefined} | ||
| > | ||
| <Avatar | ||
| class="size-6 shrink-0" | ||
| classList={{ | ||
| "ring-2 ring-foreground": location.pathname === | ||
| `/@${account.username}`, | ||
| }} | ||
|
Comment on lines
+123
to
+136
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Keep the Profile nav item active on subroutes. Currently the profile item is only active on the exact ✅ Proposed fix- {(account) => (
+ {(account) => {
+ const profilePath = `/@${account.username}`;
+ const profileActive = isActive(profilePath);
+ return (
<nav
class="fixed sm:flex sm:flex-col z-50 border-border bg-background/95 bottom-0 left-0 right-0 border-t sm:sticky sm:top-0 sm:h-screen sm:w-16 sm:shrink-0 sm:border-t-0 sm:border-r lg:w-50 sm:py-3"
role="navigation"
aria-label={t`Main navigation`}
>
@@
<li class="flex-1 sm:flex-none sm:w-full">
<A
- href={`/@${account.username}`}
+ href={profilePath}
class="flex items-center justify-center py-3 sm:p-3 lg:justify-start lg:gap-3 lg:rounded-full lg:hover:bg-accent"
aria-label={t`Profile`}
- aria-current={location.pathname === `/@${account.username}`
- ? "page"
- : undefined}
+ aria-current={profileActive ? "page" : undefined}
>
<Avatar
class="size-6 shrink-0"
classList={{
- "ring-2 ring-foreground": location.pathname ===
- `/@${account.username}`,
+ "ring-2 ring-foreground": profileActive,
}}
>
@@
- </nav>
- )}
+ </nav>
+ );}}🤖 Prompt for AI Agents |
||
| > | ||
| <AvatarImage src={account.avatarUrl} alt={account.username} /> | ||
| <AvatarFallback> | ||
| {account.username.slice(0, 2).toUpperCase()} | ||
| </AvatarFallback> | ||
| </Avatar> | ||
| <span class="hidden lg:inline">{t`Profile`}</span> | ||
| </A> | ||
| </li> | ||
| </ul> | ||
| <div class="hidden sm:flex w-full px-2 mt-5"> | ||
| <button | ||
| type="button" | ||
| onClick={openNoteCompose} | ||
| class="flex lg:flex-1 items-center justify-center p-3 rounded-full bg-primary text-primary-foreground transition-colors hover:bg-primary/90" | ||
| aria-label={t`Create Note`} | ||
| > | ||
| <IconSquarePen class="size-5" aria-hidden="true" /> | ||
| </button> | ||
| </div> | ||
|
Comment on lines
+147
to
+156
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The new unified navigation is a significant improvement. However, it seems the "Create Article" functionality, which was available in both the previous sidebar and the floating action button, has been omitted. Both the mobile and desktop versions of the new compose button only trigger the note composition modal. If this omission was unintentional, you might consider reintroducing this feature. For the desktop view, a dropdown menu on the compose button could be an effective way to offer both "Create Note" and "Create Article" options, similar to the previous implementation. |
||
| <Footer class="hidden lg:block sm:mt-auto" /> | ||
| </nav> | ||
| )} | ||
| </Show> | ||
| ); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add
rel="noopener noreferrer"to external links opened in a new tab.Both external anchors open in a new tab without
rel, which is a tabnabbing risk.🛡️ Proposed fix
<a href="https://github.com/hackers-pub/hackerspub" target="_blank" + rel="noopener noreferrer" class="underline" > ... <a href="https://www.gnu.org/licenses/agpl-3.0.en.html" target="_blank" + rel="noopener noreferrer" class="underline" >📝 Committable suggestion
🧰 Tools
🪛 Biome (2.1.2)
[error] 25-25: Avoid using target="_blank" without rel="noopener" or rel="noreferrer".
Opening external links in new tabs without rel="noopener" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noopener" attribute.
(lint/security/noBlankTarget)
[error] 34-34: Avoid using target="_blank" without rel="noopener" or rel="noreferrer".
Opening external links in new tabs without rel="noopener" is a security risk. See the explanation for more details.
Safe fix: Add the rel="noopener" attribute.
(lint/security/noBlankTarget)
🤖 Prompt for AI Agents