docs: improve demo on mobile, remove heyo (blo-1013)#2590
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
YousefED
changed the title
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe pull request enhances the demo editor's responsiveness for smaller screens, refactors the export dropdown from CSS-controlled to conditionally rendered based on state, removes external HeYo embed scripts from the demo and pricing pages, and adds Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/mantine
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-ai
@blocknote/xl-docx-exporter
@blocknote/xl-email-exporter
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
commit: |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.claude/worktrees/emoji-fixes (1)
15-16:⚠️ Potential issue | 🟠 MajorAvoid pinning a WIP subproject commit in this PR.
The referenced commit message explicitly says unresolved work remains. Pulling that pointer into this PR risks shipping incomplete behavior unrelated to BLO-1013.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/worktrees/emoji-fixes around lines 15 - 16, The PR includes a pinned subproject commit with the WIP commit message "WIP: Still need to decouple emoji height cap from GridSuggestionMenu, add native emoji source for PDF exporter, and align data sources" from the emoji-fixes worktree; remove that pinned subproject reference from the commit set (don’t update the submodule pointer to this WIP SHA), and instead either point the submodule to a stable non-WIP commit or omit the submodule change entirely so this PR only contains changes for BLO-1013; locate the submodule update in your commit that references the emoji-fixes worktree/WIP message and revert that change or replace it with a clean commit hash before pushing the branch..claude/worktrees/fix-pr-2553-port (1)
1-20:⚠️ Potential issue | 🟠 MajorRemove empty
.claude/worktreessubproject pointers from this PR.These
.claude/worktrees/entries are empty git subproject pointers with no content, zero references in the codebase, and unrelated to the PR objective (mobile demo improvements and Heyo removal). They appear to be development workspace artifacts that should not be committed. Remove all three entries (.claude/worktrees/emoji-fixes,fix-pr-2553-port, andtoggle-block-bugs-blo-1018) to keep the PR focused and auditable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/worktrees/fix-pr-2553-port around lines 1 - 20, The PR includes empty git subproject pointers under .claude/worktrees (emoji-fixes, fix-pr-2553-port, toggle-block-bugs-blo-1018) that are unrelated to the change; remove those entries from the branch by deleting the .claude/worktrees/* entries from the repo index and commit the deletion (e.g. remove the subproject pointer files or run git rm on those paths and commit), ensuring no other files are touched and the tests/snapshots changes remain; verify the removals no longer appear in git status or the PR diff before pushing.
🧹 Nitpick comments (1)
docs/app/demo/_components/EditorMenu.tsx (1)
82-121: Consider adding basic accessibility attributes to the dropdown.The click-based dropdown works well, but could benefit from accessibility improvements for screen reader users and keyboard navigation.
♿ Optional: Add ARIA attributes
- <div className="relative" ref={exportRef} onBlur={handleBlur}> + <div className="relative" ref={exportRef} onBlur={handleBlur} role="group"> <button className="flex items-center gap-1 rounded-md border border-stone-200 bg-white px-2 py-1 text-xs hover:bg-stone-50" disabled={disabled} onClick={() => setExportOpen(!exportOpen)} + aria-expanded={exportOpen} + aria-haspopup="menu" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/app/demo/_components/EditorMenu.tsx` around lines 82 - 121, The Export dropdown lacks ARIA and keyboard support: update the toggle button (the element using exportRef, exportOpen, setExportOpen) to include aria-haspopup="menu" and aria-expanded={exportOpen}, add an onKeyDown handler to open/close with Enter/Space and close on Escape, mark the popup container with role="menu" and each option button with role="menuitem" and tabIndex={0}, and ensure the handleBlur/focus management moves focus into/out of the menu so keyboard users can navigate the items and the menu closes on blur or Escape; use the existing onExport calls from the menuitem buttons to keep behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/app/demo/_components/DemoEditor.tsx`:
- Around line 190-193: The state initialization for sidebarOpen in DemoEditor
causes an SSR/client mismatch because it reads window during init; change the
useState initializer to a constant true (useState(true)) and add a useEffect in
the DemoEditor component that on mount reads window.innerWidth (>=768) and calls
setSidebarOpen accordingly; reference the existing sidebarOpen/setSidebarOpen
useState and update logic so the client adjusts after mount rather than during
SSR.
---
Outside diff comments:
In @.claude/worktrees/emoji-fixes:
- Around line 15-16: The PR includes a pinned subproject commit with the WIP
commit message "WIP: Still need to decouple emoji height cap from
GridSuggestionMenu, add native emoji source for PDF exporter, and align data
sources" from the emoji-fixes worktree; remove that pinned subproject reference
from the commit set (don’t update the submodule pointer to this WIP SHA), and
instead either point the submodule to a stable non-WIP commit or omit the
submodule change entirely so this PR only contains changes for BLO-1013; locate
the submodule update in your commit that references the emoji-fixes worktree/WIP
message and revert that change or replace it with a clean commit hash before
pushing the branch.
In @.claude/worktrees/fix-pr-2553-port:
- Around line 1-20: The PR includes empty git subproject pointers under
.claude/worktrees (emoji-fixes, fix-pr-2553-port, toggle-block-bugs-blo-1018)
that are unrelated to the change; remove those entries from the branch by
deleting the .claude/worktrees/* entries from the repo index and commit the
deletion (e.g. remove the subproject pointer files or run git rm on those paths
and commit), ensuring no other files are touched and the tests/snapshots changes
remain; verify the removals no longer appear in git status or the PR diff before
pushing.
---
Nitpick comments:
In `@docs/app/demo/_components/EditorMenu.tsx`:
- Around line 82-121: The Export dropdown lacks ARIA and keyboard support:
update the toggle button (the element using exportRef, exportOpen,
setExportOpen) to include aria-haspopup="menu" and aria-expanded={exportOpen},
add an onKeyDown handler to open/close with Enter/Space and close on Escape,
mark the popup container with role="menu" and each option button with
role="menuitem" and tabIndex={0}, and ensure the handleBlur/focus management
moves focus into/out of the menu so keyboard users can navigate the items and
the menu closes on blur or Escape; use the existing onExport calls from the
menuitem buttons to keep behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fb56ed28-447b-4a91-9cae-3439273ff9e5
📒 Files selected for processing (7)
.claude/worktrees/emoji-fixes.claude/worktrees/fix-pr-2553-port.claude/worktrees/toggle-block-bugs-blo-1018docs/app/demo/_components/DemoEditor.tsxdocs/app/demo/_components/EditorMenu.tsxdocs/app/demo/page.tsxdocs/app/pricing/page.tsx
💤 Files with no reviewable changes (2)
- docs/app/demo/page.tsx
- docs/app/pricing/page.tsx
| const [sidebarOpen, setSidebarOpen] = useState(() => { | ||
| if (typeof window === "undefined") return true; | ||
| return window.innerWidth >= 768; | ||
| }); |
There was a problem hiding this comment.
Potential hydration mismatch on mobile devices.
During SSR, window is undefined, so sidebarOpen initializes to true. On client hydration for viewports < 768px, the initial state becomes false. This can cause a React hydration mismatch warning.
Consider deferring the width check to a useEffect to avoid SSR/client state divergence:
🛠️ Suggested fix to avoid hydration mismatch
- const [sidebarOpen, setSidebarOpen] = useState(() => {
- if (typeof window === "undefined") return true;
- return window.innerWidth >= 768;
- });
+ const [sidebarOpen, setSidebarOpen] = useState(true);
+
+ useEffect(() => {
+ if (window.innerWidth < 768) {
+ setSidebarOpen(false);
+ }
+ }, []);This ensures SSR and initial client render both start with true, then adjusts based on viewport width after mount.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [sidebarOpen, setSidebarOpen] = useState(() => { | |
| if (typeof window === "undefined") return true; | |
| return window.innerWidth >= 768; | |
| }); | |
| const [sidebarOpen, setSidebarOpen] = useState(true); | |
| useEffect(() => { | |
| if (window.innerWidth < 768) { | |
| setSidebarOpen(false); | |
| } | |
| }, []); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/app/demo/_components/DemoEditor.tsx` around lines 190 - 193, The state
initialization for sidebarOpen in DemoEditor causes an SSR/client mismatch
because it reads window during init; change the useState initializer to a
constant true (useState(true)) and add a useEffect in the DemoEditor component
that on mount reads window.innerWidth (>=768) and calls setSidebarOpen
accordingly; reference the existing sidebarOpen/setSidebarOpen useState and
update logic so the client adjusts after mount rather than during SSR.
nperez0111
left a comment
nperez0111
left a comment
There was a problem hiding this comment.
go ahead & add this when you want
2 participants
Summary
Summary by CodeRabbit