fix: hide note outline for blurred memos - #6160
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 (1)
WalkthroughMemo detail tracks blurred-content visibility by memo navigation state. 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 SummaryThe PR prevents memo outlines from exposing headings while blurred content is hidden and synchronizes outline visibility with the memo reveal state.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| web/src/pages/MemoDetail.tsx | Controls reveal state by memo identity, resets it on direct or shared-memo navigation, and propagates it consistently to the memo view and sidebar. |
| web/src/components/MemoDetailSidebar/MemoDetailSidebar.tsx | Derives blur status from user tag settings and suppresses outline extraction until settings load or concealed content is revealed. |
| web/src/components/MemoView/MemoView.tsx | Supports externally controlled blur visibility while retaining local behavior for existing uncontrolled callers. |
| web/src/contexts/AppSidebarContext.tsx | Extends the memo-detail sidebar descriptor with the synchronized reveal state. |
| web/src/components/AppSidebar/AppSidebar.tsx | Passes the registered reveal state into the memo-detail sidebar. |
| web/tests/memo-detail-blur-navigation.test.tsx | Covers resetting concealed state when navigating away from and back to a previously revealed memo. |
| web/tests/memo-detail-sidebar.test.tsx | Covers hidden, revealed, and settings-loading outline states using the declared component contract. |
Reviews (5): Last reviewed commit: "Repair Blurred memo leaks headings in no..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/pages/MemoDetail.tsx`:
- Around line 36-38: Change the descriptor-registration hook in MemoDetail from
useEffect to useLayoutEffect so setMemoDetail runs before the sidebar paints,
while preserving its dependency list and leaving the unmount cleanup effect as a
regular useEffect.
🪄 Autofix
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: 9a1a65c2-633b-4f3d-a47c-820b8d77ab37
📒 Files selected for processing (7)
web/src/components/AppSidebar/AppSidebar.tsxweb/src/components/MemoDetailSidebar/MemoDetailSidebar.tsxweb/src/components/MemoView/MemoView.tsxweb/src/components/MemoView/types.tsweb/src/contexts/AppSidebarContext.tsxweb/src/pages/MemoDetail.tsxweb/tests/memo-detail-sidebar.test.tsx
| useEffect(() => { | ||
| setMemoDetail({ memo, from, readonly, onShareImageOpen }); | ||
| }, [from, memo, onShareImageOpen, readonly, setMemoDetail]); | ||
| setMemoDetail({ memo, from, readonly, onShareImageOpen, blurred, showBlurredContent }); | ||
| }, [blurred, from, memo, onShareImageOpen, readonly, setMemoDetail, showBlurredContent]); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate candidate file:"
fd -a 'MemoDetail\.tsx$' . || true
echo
echo "Show relevant file outline/size:"
if [ -f web/src/pages/MemoDetail.tsx ]; then
wc -l web/src/pages/MemoDetail.tsx
ast-grep outline web/src/pages/MemoDetail.tsx || true
echo
echo "Relevant source:"
sed -n '1,220p' web/src/pages/MemoDetail.tsx | cat -n
fi
echo
echo "Search MemoDetailSidebar/context usages:"
rg -n "MemoDetail|MemoSidebarRegistration|MemoDetailSidebar|memoDetail|useMemode|MemoDetailContext|useMemoDetail" web/src || trueRepository: usememos/memos
Length of output: 16257
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "AppSidebarContext:"
sed -n '1,80p' web/src/contexts/AppSidebarContext.tsx | cat -n
echo
echo "MemoDetailSidebar outline and blur/render sections:"
wc -l web/src/components/MemoDetailSidebar/MemoDetailSidebar.tsx
ast-grep outline web/src/components/MemoDetailSidebar/MemoDetailSidebar.tsx || true
rg -n "blurred|showBlurredContent|Heading|showBlurredContent|memo\.name|children|Outline|toc|TableOf" web/src/components/MemoDetailSidebar/MemoDetailSidebar.tsx -C 3
echo
echo "Sidebar render block:"
sed -n '340,425p' web/src/components/AppSidebar/AppSidebar.tsx | cat -n
echo
echo "MemoDetailSidebar relevant sections:"
sed -n '1,220p' web/src/components/MemoDetailSidebar/MemoDetailSidebar.tsx | cat -nRepository: usememos/memos
Length of output: 18293
Synchronize blur state before the sidebar can paint.
MemoSidebarRegistration writes memoDetail after the render, so the sidebar can still render the stale descriptor before useEffect runs. Switch this descriptor update to useLayoutEffect; keep the unmount cleanup effect as a regular useEffect.
🤖 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/pages/MemoDetail.tsx` around lines 36 - 38, Change the
descriptor-registration hook in MemoDetail from useEffect to useLayoutEffect so
setMemoDetail runs before the sidebar paints, while preserving its dependency
list and leaving the unmount cleanup effect as a regular useEffect.
f59ed46 to
36a39b7
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
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/MemoDetailSidebar/MemoDetailSidebar.tsx`:
- Line 31: Align the MemoDetailSidebar prop contract with the regression test by
choosing one supported blur API: update the test to provide the required blur
metadata and pass showBlurredContent, or add and consistently handle an explicit
blurred prop in the Props definition and component logic. Ensure the test
compiles and the memo is treated as blurred at runtime.
- Around line 81-85: Update MemoDetailSidebar’s blurred calculation to wait for
resolved user tag settings, using isUserSettingsInitialized to prevent headings
from being extracted while settings are still loading. Preserve the existing
showBlurredContent behavior once settings are initialized, and use the existing
resolved blur state if MemoDetail already provides one.
🪄 Autofix
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: 50bbf517-a5e2-4e13-a26c-e4db7b1fe72a
📒 Files selected for processing (7)
web/src/components/AppSidebar/AppSidebar.tsxweb/src/components/MemoDetailSidebar/MemoDetailSidebar.tsxweb/src/components/MemoView/MemoView.tsxweb/src/components/MemoView/types.tsweb/src/contexts/AppSidebarContext.tsxweb/src/pages/MemoDetail.tsxweb/tests/memo-detail-sidebar.test.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- web/tests/memo-detail-sidebar.test.tsx
- web/src/contexts/AppSidebarContext.tsx
- web/src/components/MemoView/types.ts
- web/src/components/MemoView/MemoView.tsx
- web/src/pages/MemoDetail.tsx
36a39b7 to
f59ed46
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
1 participant
Fixes #6158
Hide the note outline while blurred content is hidden, and restore it after reveal.
Added regression coverage.