feat/web: add memo detail shortcut button - #6138
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughAdds the exported Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryAdds a
Confidence Score: 5/5Safe to merge — the button is correctly gated, navigation state is properly forwarded, and the change is isolated to a small new component with a test. Both previously flagged issues are resolved: parentPage is forwarded as router state and the button is hidden on the detail page via the isInMemoDetailPage guard. The change is small, well-tested, and does not touch any critical paths. Files Needing Attention: No files require special attention.
|
| Filename | Overview |
|---|---|
| web/src/components/MemoView/components/MemoHeader.tsx | Adds MemoDetailLinkButton (exported) with correct parentPage state and isInMemoDetailPage guard; minor i18n key reuse and missing tooltip |
| web/tests/memo-detail-link-button.test.tsx | New focused test verifying href and router state — correctly uses MemoryRouter + fireEvent to confirm navigation state is forwarded |
Reviews (2): Last reviewed commit: "feat(web): add memo detail shortcut butt..." | Re-trigger Greptile
There was a problem hiding this comment.
Pull request overview
This PR adds a small “open memo detail” shortcut in the memo header UI to let users jump directly to the memo detail route from a memo card, and introduces a unit test validating the generated link.
Changes:
- Add a new
MemoDetailLinkButtonicon-link toMemoHeaderthat routes to/${memo.name}. - Show the button on mobile and on memo hover for larger screens via Tailwind responsive/hover classes.
- Add a focused test to ensure the detail link href is generated correctly.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| web/src/components/MemoView/components/MemoHeader.tsx | Adds the new memo detail shortcut button component and renders it in the header actions. |
| web/tests/memo-detail-link-button.test.tsx | Adds a unit test asserting the shortcut link targets the correct memo detail URL. |
Suppressed comments (1)
web/src/components/MemoView/components/MemoHeader.tsx:123
MemoDetailLinkButtonshould forward the "from" navigation context (derived fromparentPage) into theLink'sstateso MemoDetail can retain navigation context (locationState?.from). Right now the button drops that state entirely.
export const MemoDetailLinkButton: React.FC<{ memoName: string }> = ({ memoName }) => (
<Link
aria-label="Open memo"
className="h-6 w-6 flex sm:hidden sm:group-hover:flex justify-center items-center cursor-pointer transition-colors text-muted-foreground hover:text-foreground"
to={`/${memoName}`}
viewTransition
>
<ArrowUpRightIcon className="w-4 h-4" />
</Link>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /> | ||
| )} | ||
|
|
||
| <MemoDetailLinkButton memoName={memo.name} /> |
|
|
||
| export const MemoDetailLinkButton: React.FC<{ memoName: string }> = ({ memoName }) => ( | ||
| <Link | ||
| aria-label="Open memo" |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/src/components/MemoView/components/MemoHeader.tsx`:
- Line 117: Update the accessible name in MemoHeader to use the existing
useTranslate hook and an appropriate translation key instead of hard-coded “Open
memo” text. Add or reuse the corresponding localized string, then update
memo-detail-link-button.test.tsx to assert the translated accessible name.
- Line 118: Update the MemoHeader link className using the existing cn() helper:
replace the sm:hidden display behavior with opacity-based hiding that preserves
keyboard focusability, and reveal it on both sm:group-hover and
sm:focus-visible. Split the classes into Biome-formatted lines within the
140-character limit while preserving the existing styling and interaction
classes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b47c594d-e494-4518-af38-b93b6a010b85
📒 Files selected for processing (2)
web/src/components/MemoView/components/MemoHeader.tsxweb/tests/memo-detail-link-button.test.tsx
|
|
||
| export const MemoDetailLinkButton: React.FC<{ memoName: string }> = ({ memoName }) => ( | ||
| <Link | ||
| aria-label="Open memo" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Localize the accessible name.
aria-label="Open memo" is hard-coded in English, while MemoHeader already uses useTranslate. In other locales, screen readers receive English text. Use a translation key and update web/tests/memo-detail-link-button.test.tsx to assert the localized name.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/components/MemoView/components/MemoHeader.tsx` at line 117, Update
the accessible name in MemoHeader to use the existing useTranslate hook and an
appropriate translation key instead of hard-coded “Open memo” text. Add or reuse
the corresponding localized string, then update memo-detail-link-button.test.tsx
to assert the translated accessible name.
454ee66 to
e84ff45
Compare
Summary
Adds a small hover quick action for opening a memo detail page directly from the memo header.
This uses an
ArrowUpRighticon link next to the existing inline actions, visible on mobile and on memo hover for larger screens. It is intended as a lightweight shortcut for users who frequently jump from the feed into a memo detail view.This is related to #6055, which asks for quicker inline memo actions without going through the overflow menu.
Changes
MemoDetailLinkButtontoMemoHeaderChecks
./node_modules/.bin/vitest run tests/memo-detail-link-button.test.tsxpnpm lintpnpm release