remove filter far from search results and add return home button#38
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a new HomeLink UI component and restructures the application's routing hierarchy by consolidating filtered content routes (/, /best, /new) under a parent Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
Deploying hnfeed with
|
| Latest commit: |
cbfa28d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d3d81888.hnfeed.pages.dev |
| Branch Preview URL: | https://filterbarclean.hnfeed.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/client/src/routes/_filtered/index.tsx (1)
10-11: Stale TODO comment should be removed.The TODO mentions replacing placeholder data logic, but
useTopStoriesis already implemented and in use. Consider removing this outdated comment.🧹 Suggested fix
-//TODO: Get data logic needs to replace placeholder -// - function RouteComponent() {apps/client/src/routes/author/$authorid.tsx (1)
51-77:HomeLinkis missing from the loading state.The
HomeLinkcomponent is rendered at line 81 in the loaded state, but the loading skeleton (lines 51-77) returns early without it. This creates an inconsistent UX where users cannot navigate home while the page is loading.🛠️ Proposed fix
if (isLoading) { return ( <div className={style.authorPage}> + <HomeLink /> <div className={style.profileCard}>
🤖 Fix all issues with AI agents
In `@apps/client/src/components/ui/home-link/home-link.tsx`:
- Around line 5-11: The HomeLink component hardcodes the English string "Home";
update HomeLink to use react-i18next instead: import and call useTranslation
inside the HomeLink function, replace the literal span text with the translation
key (e.g., t('home') or a namespaced key like t('common:home')), and keep the
Link, ArrowLeft and style.homeLink usage intact so only the display text changes
to a translated value.
In `@apps/client/src/routes/_filtered.tsx`:
- Around line 9-25: FilteredLayout triggers redundant navigation: remove the
handleFilter navigation flow and let FilterBar's Link-driven MenuItem navigation
be the single source of truth. Delete the handleFilter function and the
useNavigate usage in FilteredLayout, and stop passing the handleChange prop (or
pass a no-op) to FilterBar so navigation occurs only via FilterBar's
Link/MenuItem; update any references to props.handleChange/handleSelect in
FilterBar only if you choose the alternate approach.
| export function HomeLink() { | ||
| return ( | ||
| <Link to="/" className={style.homeLink}> | ||
| <ArrowLeft size={16} /> | ||
| <span>Home</span> | ||
| </Link> | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider internationalising the "Home" text.
The string "Home" on line 9 is hardcoded in English. Other components in this codebase (e.g., author/$authorid.tsx) use react-i18next for translations. For consistency, consider using the translation hook.
♻️ Suggested refactor
import { Link } from '@tanstack/react-router';
import { ArrowLeft } from 'lucide-react';
+import { useTranslation } from 'react-i18next';
import style from './home-link.module.css';
export function HomeLink() {
+ const { t } = useTranslation();
return (
<Link to="/" className={style.homeLink}>
<ArrowLeft size={16} />
- <span>Home</span>
+ <span>{t('common.home')}</span>
</Link>
);
}🤖 Prompt for AI Agents
In `@apps/client/src/components/ui/home-link/home-link.tsx` around lines 5 - 11,
The HomeLink component hardcodes the English string "Home"; update HomeLink to
use react-i18next instead: import and call useTranslation inside the HomeLink
function, replace the literal span text with the translation key (e.g.,
t('home') or a namespaced key like t('common:home')), and keep the Link,
ArrowLeft and style.homeLink usage intact so only the display text changes to a
translated value.
| function FilteredLayout() { | ||
| const navigate = useNavigate(); | ||
|
|
||
| function handleFilter(filter: PrimaryFilter) { | ||
| if (filter === 'hot') { | ||
| navigate({ to: '/' }); | ||
| } else { | ||
| navigate({ to: `/${filter}` }); | ||
| } | ||
| } | ||
|
|
||
| return ( | ||
| <> | ||
| <FilterBar handleChange={handleFilter} /> | ||
| <Outlet /> | ||
| </> | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat apps/client/src/components/ui/filter-bar/filter-bar.tsxRepository: nahmanmate/hacker-news
Length of output: 2631
Remove redundant navigation between FilterBar and FilteredLayout.
The FilterBar component has a Link component inside each MenuItem that triggers navigation directly. Additionally, when a MenuItem is selected, handleSelect calls props.handleChange(filter), which invokes handleFilter in FilteredLayout and triggers a second navigate() call. This creates two navigation events for a single user interaction, which could lead to race conditions or unexpected behaviour.
Consider either:
- Removing the
Linkwrapper insideFilterBar'sMenuItemand relying solely onhandleChange, or - Removing the
handleChangecallback here and lettingFilterBarhandle navigation internally via itsLinkcomponents.
🤖 Prompt for AI Agents
In `@apps/client/src/routes/_filtered.tsx` around lines 9 - 25, FilteredLayout
triggers redundant navigation: remove the handleFilter navigation flow and let
FilterBar's Link-driven MenuItem navigation be the single source of truth.
Delete the handleFilter function and the useNavigate usage in FilteredLayout,
and stop passing the handleChange prop (or pass a no-op) to FilterBar so
navigation occurs only via FilterBar's Link/MenuItem; update any references to
props.handleChange/handleSelect in FilterBar only if you choose the alternate
approach.
1 participant
Summary by CodeRabbit
Release Notes
New Features
Navigation
✏️ Tip: You can customize this high-level summary in your review settings.