-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
feat: add word and character count footer to page editor #13978
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
base: canary
Are you sure you want to change the base?
Conversation
|
|
WalkthroughAdded word and character count metrics to the document editor with a bottom display bar. Adjusted full-screen layout condition in the editor. Applied minor whitespace formatting to mobile appearance configuration files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
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.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/frontend/core/src/mobile/dialogs/setting/appearance/language.tsx (1)
41-41: Unrelated whitespace change.This trailing newline is unrelated to the PR's objective of adding word and character count functionality. Consider removing formatting-only changes from feature PRs to keep the diff focused.
packages/frontend/core/src/mobile/dialogs/setting/appearance/index.tsx (1)
3-4: Unrelated whitespace changes.These blank lines are unrelated to the PR's objective of adding word and character count functionality. Consider removing formatting-only changes from feature PRs to keep the diff focused.
Also applies to: 10-10
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/frontend/core/src/components/page-detail-editor.tsx(3 hunks)packages/frontend/core/src/mobile/dialogs/setting/appearance/index.tsx(1 hunks)packages/frontend/core/src/mobile/dialogs/setting/appearance/language.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/frontend/core/src/components/page-detail-editor.tsx (1)
packages/frontend/core/src/blocksuite/block-suite-editor/blocksuite-editor.tsx (1)
BlockSuiteEditor(280-376)
🪛 Biome (2.1.2)
packages/frontend/core/src/components/page-detail-editor.tsx
[error] 29-29: expected : but instead found [
Remove [
(parse)
[error] 29-29: expected , but instead found ;
Remove ;
(parse)
[error] 29-29: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 30-30: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 32-32: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🔇 Additional comments (2)
packages/frontend/core/src/components/page-detail-editor.tsx (2)
71-71: Verify the inverted full-screen layout logic.The condition for applying the
full-screenclass has been inverted from!isSharedMode && fullWidthLayouttoisSharedMode && fullWidthLayout. This means:
- Before: Full-screen applied when NOT in shared mode
- After: Full-screen applied when in shared mode
This change is not mentioned in the PR description and appears unrelated to adding word/character counts. If unintentional, this is a major bug that will break the layout behavior.
Please confirm whether this logic change is intentional. If not, revert it:
className={clsx(styles.editor, { - 'full-screen': isSharedMode && fullWidthLayout, + 'full-screen': !isSharedMode && fullWidthLayout, 'is-public': isSharedMode, })}
67-69: Layout wrapper is properly implemented and safe to merge.The flex column wrapper with the status bar is a standard React layout pattern. The CSS module defines
.editorwithflex: 1, which correctly expands to fill available space within the flex container. No conflicting styles or DOM structure dependencies were found:
- Vanilla-extract CSS is scoped and compatible with inline styles
- Parent components (detail-page, share-page, mobile-detail-page) import PageDetailEditor as an encapsulated component with no direct DOM queries
- No test files or selectors target the internal structure
| const [wordCount, setWordCount] = useState(0); | ||
| const [charCount, setCharCount] = useState(0); |
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.
Critical: Syntax error and missing import.
The useState hooks are declared outside the function body (before the function parameters that start at line 41). This is a syntax error that will prevent the code from running. Additionally, useState is not imported from React.
Apply this diff to fix the syntax error and add the missing import:
import { useLiveData, useService } from '@toeverything/infra';
import clsx from 'clsx';
-import { useEffect } from 'react';
+import { useEffect, useState } from 'react';
import type { AffineEditorContainer } from '../blocksuite/block-suite-editor';Then move the useState declarations inside the function body:
export const PageDetailEditor = ({
- const [wordCount, setWordCount] = useState(0);
- const [charCount, setCharCount] = useState(0);
-
- useEffect(() => {
- const text = document.body.innerText || "";
- const words = text.trim().split(/\s+/).filter(Boolean);
-
- setCharCount(text.length);
- setWordCount(words.length);
- });
-
-
onLoad,
readonly,
}: PageDetailEditorProps) => {
+ const [wordCount, setWordCount] = useState(0);
+ const [charCount, setCharCount] = useState(0);
+
const editor = useService(EditorService).editor;📝 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 [wordCount, setWordCount] = useState(0); | |
| const [charCount, setCharCount] = useState(0); | |
| import { useLiveData, useService } from '@toeverything/infra'; | |
| import clsx from 'clsx'; | |
| import { useEffect, useState } from 'react'; | |
| import type { AffineEditorContainer } from '../blocksuite/block-suite-editor'; | |
| export const PageDetailEditor = ({ | |
| onLoad, | |
| readonly, | |
| }: PageDetailEditorProps) => { | |
| const [wordCount, setWordCount] = useState(0); | |
| const [charCount, setCharCount] = useState(0); | |
| useEffect(() => { | |
| const text = document.body.innerText || ""; | |
| const words = text.trim().split(/\s+/).filter(Boolean); | |
| setCharCount(text.length); | |
| setWordCount(words.length); | |
| }); | |
| const editor = useService(EditorService).editor; |
🧰 Tools
🪛 Biome (2.1.2)
[error] 29-29: expected : but instead found [
Remove [
(parse)
[error] 29-29: expected , but instead found ;
Remove ;
(parse)
[error] 29-29: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 30-30: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🤖 Prompt for AI Agents
In packages/frontend/core/src/components/page-detail-editor.tsx around lines
29-30, the two useState declarations are placed outside the component function
and useState is not imported from React; move the const [wordCount,
setWordCount] and const [charCount, setCharCount] declarations into the
component function body (i.e., after the function starts around line 41) and add
useState to the React import at the top of the file (import React, { useState,
... } from 'react') so the hooks are used legally inside the component and the
hook symbol is defined.
| useEffect(() => { | ||
| const text = document.body.innerText || ""; | ||
| const words = text.trim().split(/\s+/).filter(Boolean); | ||
|
|
||
| setCharCount(text.length); | ||
| setWordCount(words.length); | ||
| }); |
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.
🧩 Analysis chain
Critical: useEffect captures wrong content and causes infinite re-renders.
This implementation has multiple critical issues:
- Wrong text source:
document.body.innerTextcaptures ALL text on the entire page, including navigation, sidebars, and other UI elements—not just the editor content. - Missing dependencies: The useEffect has no dependency array, so it runs on every render, causing severe performance issues and potential infinite re-render loops.
- Location error: This hook is also declared outside the function body (see previous comment).
After moving the hooks inside the function body (see previous comment), you need to extract text from the editor's document instead. Here's the correct approach:
+ useEffect(() => {
+ // Extract text from the editor's BlockSuite document
+ const text = editor.doc.blockSuiteDoc.root?.children
+ .map((block: any) => block.text?.toString() || '')
+ .join(' ')
+ .trim() || '';
+
+ const words = text.split(/\s+/).filter(Boolean);
+ setWordCount(words.length);
+ setCharCount(text.length);
+ }, [editor.doc.blockSuiteDoc.root]);Note: You may need to verify the exact API for extracting text from BlockSuite documents.
Run this script to find examples of how to extract text from BlockSuite documents in the codebase:
🏁 Script executed:
#!/bin/bash
# Search for text extraction patterns from BlockSuite documents
rg -nP --type=ts -C3 'blockSuiteDoc\.(root|text|innerText)'Length of output: 686
I need to gather more information about how text extraction works in this codebase. Let me search for actual implementations:
Let me search the codebase more comprehensively for text extraction patterns and the context of this editor:
Now let me examine the actual page-detail-editor.tsx file and search for existing implementations in the codebase:
Let me search for actual text extraction in the codebase and look at how this file is structured:
Also search for word count or character count patterns already in the code:
---
🏁 Script executed:
```shell
#!/bin/bash
# Get full context of page-detail-editor.tsx around the problematic lines
cat packages/frontend/core/src/components/page-detail-editor.tsx | head -50
Length of output: 1489
🏁 Script executed:
#!/bin/bash
# Search for actual text extraction from blocks
rg -nP --type=ts -C5 'model\.text|\.toString\(\).*text|getText|innerText' packages/frontend/core/src/ | head -100Length of output: 8950
🏁 Script executed:
#!/bin/bash
# Search for word count or character count logic in the codebase
rg -nP --type=ts -C3 'setWordCount|setCharCount|word.*count|char.*count' packages/frontend/core/src/Length of output: 2147
Critical: Three syntax and logic errors in useEffect hook.
This implementation has multiple critical issues that must be fixed:
-
Misplaced hook: The
useEffectis declared outside the function body (before destructuringonLoadandreadonly), which causes a syntax error. Hooks must be inside the function body. -
Missing dependency array: Without a dependency array, the effect runs on every render, causing severe performance issues and potential infinite re-render loops.
-
Wrong text source:
document.body.innerTextcaptures ALL text on the entire page (navigation, sidebars, other UI elements), not the editor content.
Move the hook inside the function body and extract text from the editor's document blocks. Based on patterns in the codebase (e.g., comment-provider.ts, markdown-utils.ts), blocks store text in the .text property:
+ useEffect(() => {
+ const blocks = editor.doc.blockSuiteDoc.root?.children || [];
+ const text = blocks
+ .map((block: any) => block.model?.text?.toString?.() || '')
+ .join('\n')
+ .trim();
+
+ const words = text.split(/\s+/).filter(Boolean);
+ setCharCount(text.length);
+ setWordCount(words.length);
+ }, [editor.doc.blockSuiteDoc.root]);Verify the exact BlockSuite API for accessing block text in your version, as .model.text may need adjustments based on your schema.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (2.1.2)
[error] 32-32: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🤖 Prompt for AI Agents
In packages/frontend/core/src/components/page-detail-editor.tsx around lines 32
to 38, the useEffect is misplaced, missing a dependency array, and reads
document.body.innerText instead of the editor content; move the useEffect inside
the component function body after destructuring onLoad and readonly (so hooks
are valid), change the source of text to derive it from the editor's blocks
(e.g., iterate the editor document/blocks and concatenate each block.text or
block.model.text depending on your BlockSuite schema), compute char and word
counts from that concatenated text, and add an appropriate dependency array
(e.g., [editor, onLoad, readonly] or a content/blocks prop/state) so the effect
only runs when the editor or its content changes.
| <div | ||
| style={{ | ||
| padding: "8px", | ||
| borderTop: "1px solid #ccc", | ||
| fontSize: "12px", | ||
| opacity: 0.8 | ||
| }} | ||
| > | ||
| Words: {wordCount} | Characters: {charCount} | ||
| </div> |
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.
🛠️ Refactor suggestion | 🟠 Major
Use i18n for text and CSS modules for styling.
The metrics bar has several issues:
-
Hardcoded English text: The labels "Words" and "Characters" are hardcoded instead of using the i18n system that's already available in the codebase (see similar usage in other files).
-
Inline styles: Using inline styles instead of the CSS modules pattern already established in this file (see
stylesimport at line 12). -
No accessibility: Missing semantic HTML or ARIA labels for screen readers.
-
Add the translations to your i18n files, then use them:
import { useI18n } from '@affine/i18n';
// Inside the component:
const t = useI18n();
// In the JSX:
<div className={styles.metricsBar}>
{t['com.affine.editor.metrics.words']()}: {wordCount} |
{t['com.affine.editor.metrics.characters']()}: {charCount}
</div>- Move styles to the CSS module file (
page-detail-editor.css):
.metricsBar {
padding: 8px;
border-top: 1px solid var(--affine-border-color);
font-size: 12px;
opacity: 0.8;
}- Add ARIA label for accessibility:
<div className={styles.metricsBar} role="status" aria-live="polite">🤖 Prompt for AI Agents
In packages/frontend/core/src/components/page-detail-editor.tsx around lines
82-91, replace the hardcoded metrics bar text and inline styles with i18n and
CSS-module usage and add accessibility attributes: import and call useI18n to
obtain t and replace "Words" and "Characters" with t keys (add corresponding
keys to the repo i18n files, e.g. com.affine.editor.metrics.words and
.characters), remove the inline style object and use a className (e.g.
styles.metricsBar) defined in page-detail-editor.css with the provided rules
(use CSS variable for border color), and add role="status" and
aria-live="polite" to the div for screen-reader accessibility.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## canary #13978 +/- ##
==========================================
- Coverage 57.17% 56.62% -0.56%
==========================================
Files 2757 2757
Lines 137719 137719
Branches 21144 21037 -107
==========================================
- Hits 78745 77986 -759
- Misses 56748 57503 +755
- Partials 2226 2230 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This PR adds a live word and character count footer to the PageDetailEditor.
The feature:
This improves the editing experience with minimal UI impact.
Summary by CodeRabbit
New Features
Style