✨Comment side panel#2279
Merged
Merged
Conversation
This was
linked to
issues
May 7, 2026
Closed
Contributor
|
Size Change: +11.3 kB (+0.26%) Total Size: 4.32 MB 📦 View Changed
|
AntoLC
force-pushed
the
feat/comment-side-panel
branch
2 times, most recently
from
7bd93dd to
137a9bc
Compare
Contributor
|
🚀 Preview will be available at https://2279-docs.ppr-docs.beta.numerique.gouv.fr/ You can use the existing account with these credentials:
You can also create a new account if you want to. Once this Pull Request is merged, the preview will be destroyed. |
AntoLC
force-pushed
the
feat/comment-side-panel
branch
2 times, most recently
from
d576c8b to
59b8d3a
Compare
This comment was marked as resolved.
This comment was marked as resolved.
robin-lecomte
added
the
⚠️ Needs corrections
Collaborator
Author
Your review is a PR comment, so we cannot resolve it or answer part of it, better to move your review here, it will give more flexibility: https://docs.numerique.gouv.fr/docs/24227219-8da8-441f-8edb-58f8c444873c/ |
AntoLC
force-pushed
the
feat/comment-side-panel
branch
6 times, most recently
from
ae9b169 to
befff14
Compare
AntoLC
force-pushed
the
feat/comment-side-panel
branch
from
befff14 to
f6dc394
Compare
Collaborator
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
WalkthroughThis PR implements a comment side panel UI for the Impress document editor, complemented by backend support for unresolving threads and comprehensive frontend refactoring. The backend adds an Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/frontend/apps/e2e/__tests__/app-impress/utils-editor.ts (1)
55-63: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winReuse
tryFocusEditorContentinwriteInEditorto avoid drift.
writeInEditorduplicates the same focus fallback logic already implemented intryFocusEditorContent. Reusing the helper keeps focus behavior consistent across tests.Proposed refactor
export const writeInEditor = async ({ page, text, }: { page: Page; text: string; }) => { - const editor = await getEditor({ page }); - if ( - (await editor.locator('.bn-trailing-block.ProseMirror-widget').count()) > 0 - ) { - await editor.locator('.bn-trailing-block.ProseMirror-widget').click(); - } else { - await editor.click(); - } + const editor = await tryFocusEditorContent({ page }); await editor .locator('.bn-block-outer:last-child') .last() .locator('.bn-inline-content:last-child') .last() .fill(text); return editor; };🤖 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 `@src/frontend/apps/e2e/__tests__/app-impress/utils-editor.ts` around lines 55 - 63, The writeInEditor helper duplicates focus/fallback logic; replace the manual focus block in writeInEditor (the getEditor + locator('.bn-trailing-block.ProseMirror-widget') check and click fallback) by calling the existing tryFocusEditorContent helper to set focus consistently, then proceed with the typing actions; locate writeInEditor and tryFocusEditorContent (and getEditor) in the same test utils file and remove the duplicated locator/click code so focus behavior is delegated to tryFocusEditorContent.
🤖 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 `@src/backend/core/tests/documents/test_api_documents_threads.py`:
- Around line 1264-1265: The test creates a resolved thread and a comment with
null actors so it doesn't guarantee resolved_at/resolved_by are populated before
exercising the "unresolve" behavior; update the setup for ThreadFactory and
CommentFactory so resolved_at and resolved_by are explicitly non-null (e.g.,
create the thread with a non-null creator and/or set resolved_at and resolved_by
on the ThreadFactory instance or create a resolving CommentFactory with a
non-null user) before calling the unresolve path so the assertions that
resolved_at and resolved_by are cleared actually validate clearing; refer to
ThreadFactory, CommentFactory, resolved_at and resolved_by to locate and adjust
the setup.
In `@src/frontend/apps/e2e/__tests__/app-impress/doc-table-content.spec.ts`:
- Around line 42-53: Replace brittle exact-pixel CSS assertions for indentation
(the expect(...).toHaveCSS checks on level1, level2, level3) with
relative/monotonic assertions: read the numeric padding/padding-left values for
elements level1, level2, level3 (e.g., via page.evaluate/getComputedStyle in the
test) and assert level1 < level2 and level2 < level3; keep the visibility and
aria-selected checks (editorLevel1/editorLevel2 visibility assertions and
aria-selected expectations) intact so only the fixed-value regex checks are
removed and replaced by numeric comparisons between level1/level2/level3.
In
`@src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteEditor.tsx`:
- Around line 304-306: The inline composer is always mounted because
FloatingComposerController is only gated by showComments; update the condition
in BlockNoteEditor to also check that the comments sidebar is not open (use the
existing isCommentSideBarOpen flag) so FloatingComposerController is rendered
only when showComments && !isCommentSideBarOpen, mirroring the
FloatingThreadController rendering logic.
In
`@src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/DocsThreadStoreAuth.tsx`:
- Around line 61-63: canUnresolveThread currently returns true unconditionally,
so users without permission see the unresolve action; change
canUnresolveThread(thread: ClientThreadData) to return a real permission check
(e.g., return Boolean(thread.canUnresolve) or call the app's ability checker
like this.ability.can('unresolve', thread) or
this.store.currentUser?.canUnresolveThread(thread)), ensuring you reference the
ClientThreadData argument and the canUnresolveThread function so the UI is gated
by the actual permission.
In
`@src/frontend/apps/impress/src/features/docs/doc-table-content/components/TableContentSideBar.tsx`:
- Around line 62-83: The cleanup currently removes the scroll listener but
doesn't clear the pending debounce timer stored in the local timeout variable,
allowing the delayed callback (which calls handleScroll / setHeadingIdHighlight)
to run after unmount; update the effect cleanup to also check and clear timeout
(e.g., if (timeout) clearTimeout(timeout)) so the scheduled
setHeadingIdHighlight/handleScroll will not fire after the component is torn
down — locate the timeout variable and scrollFn in TableContentSideBar and add
the cancel logic to the returned cleanup along with removing the listener for
MAIN_LAYOUT_ID.
In
`@src/frontend/apps/impress/src/features/docs/docs-grid/components/DocsGrid.tsx`:
- Line 100: Replace the use of isDesktop in the responsive max-height
calculation inside the DocsGrid component with the new breakpoint flag
isLargeScreen: update the template expression `$maxHeight={`calc(100vh - 52px -
${isDesktop ? '2rem' : '0rem'})`}` to reference isLargeScreen instead so the
max-height/scroll behavior follows the migrated breakpoint logic used by
DocsGrid and related responsive code paths.
In
`@src/frontend/apps/impress/src/features/left-panel/components/LeftPanelFavoriteItem.tsx`:
- Line 17: The actions are hidden based solely on
useResponsiveStore().isLargeScreen which also hides them on tablets where hover
isn't reliable; update the visibility condition in LeftPanelFavoriteItem so
actions remain visible on tablet/touch layouts by factoring in touch/hover
capability (e.g., detect hover support via window.matchMedia('(hover: none)') or
a new flag from useResponsiveStore like supportsHover/isTouch) and change the
check from just isLargeScreen to something like (isLargeScreen ||
!supportsHover) so touch devices always show the pinned actions.
In
`@src/frontend/apps/impress/src/features/right-panel/components/RightPanelCollapseButton.tsx`:
- Around line 16-24: RightPanelCollapseButton's useEffect should seed hasThreads
from the store snapshot before relying on the subscription; synchronously read
the current threads snapshot from threadStore (using its sync getter API such as
threadStore.get()/threadStore.getSnapshot()/threadStore.value — whichever is the
store's accessor) and call setHasThreads(snapshot.size > 0) immediately, then
call threadStore.subscribe(...) and return the unsubscribe; keep the existing
setHasThreads call inside the subscriber so future updates still update state.
In `@src/frontend/apps/impress/src/layouts/MainLayout.tsx`:
- Around line 56-57: MainContent still uses isDesktop for padding/background
while MainLayout now exposes isLargeScreen; update MainContent (and any spots at
the other occurrence) to use isLargeScreen instead of isDesktop so tablet uses
large-screen styling when isLargeScreen is true. Locate references to isDesktop
inside the MainContent component and any prop names passed from MainLayout,
replace them with isLargeScreen (and rename props if needed) and ensure
padding/background branching logic uses isLargeScreen throughout to avoid the
tablet layout drift.
---
Outside diff comments:
In `@src/frontend/apps/e2e/__tests__/app-impress/utils-editor.ts`:
- Around line 55-63: The writeInEditor helper duplicates focus/fallback logic;
replace the manual focus block in writeInEditor (the getEditor +
locator('.bn-trailing-block.ProseMirror-widget') check and click fallback) by
calling the existing tryFocusEditorContent helper to set focus consistently,
then proceed with the typing actions; locate writeInEditor and
tryFocusEditorContent (and getEditor) in the same test utils file and remove the
duplicated locator/click code so focus behavior is delegated to
tryFocusEditorContent.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e1db1afc-ca4c-4d0a-8ee2-5137a73c66a1
⛔ Files ignored due to path filters (4)
src/frontend/apps/impress/src/assets/icons/ui-kit/bulleted-list.svgis excluded by!**/*.svgsrc/frontend/apps/impress/src/assets/icons/ui-kit/filter-notification.svgis excluded by!**/*.svgsrc/frontend/apps/impress/src/assets/icons/ui-kit/filter_list.svgis excluded by!**/*.svgsrc/frontend/apps/impress/src/assets/icons/ui-kit/x-mark.svgis excluded by!**/*.svg
📒 Files selected for processing (42)
CHANGELOG.mdsrc/backend/core/api/viewsets.pysrc/backend/core/models.pysrc/backend/core/tests/documents/test_api_documents_threads.pysrc/frontend/apps/e2e/__tests__/app-impress/doc-comments.spec.tssrc/frontend/apps/e2e/__tests__/app-impress/doc-table-content.spec.tssrc/frontend/apps/e2e/__tests__/app-impress/left-panel.spec.tssrc/frontend/apps/e2e/__tests__/app-impress/utils-editor.tssrc/frontend/apps/impress/src/components/modal/ButtonCloseModal.tsxsrc/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteEditor.tsxsrc/frontend/apps/impress/src/features/docs/doc-editor/components/DocEditor.tsxsrc/frontend/apps/impress/src/features/docs/doc-editor/components/comments/CommentSideBar.tsxsrc/frontend/apps/impress/src/features/docs/doc-editor/components/comments/CommentToolbarButton.tsxsrc/frontend/apps/impress/src/features/docs/doc-editor/components/comments/DocsThreadStore.tsxsrc/frontend/apps/impress/src/features/docs/doc-editor/components/comments/DocsThreadStoreAuth.tsxsrc/frontend/apps/impress/src/features/docs/doc-editor/components/comments/index.tssrc/frontend/apps/impress/src/features/docs/doc-editor/components/comments/styles.tsxsrc/frontend/apps/impress/src/features/docs/doc-editor/components/comments/useCommentSidebarStore.tssrc/frontend/apps/impress/src/features/docs/doc-editor/hook/useHeadings.tsxsrc/frontend/apps/impress/src/features/docs/doc-editor/stores/index.tssrc/frontend/apps/impress/src/features/docs/doc-editor/stores/usePanelEditorStore.tsxsrc/frontend/apps/impress/src/features/docs/doc-editor/styles.tsxsrc/frontend/apps/impress/src/features/docs/doc-header/components/DocTitle.tsxsrc/frontend/apps/impress/src/features/docs/doc-header/components/FloatingBar.tsxsrc/frontend/apps/impress/src/features/docs/doc-search/components/DocSearchModal.tsxsrc/frontend/apps/impress/src/features/docs/doc-table-content/components/Heading.tsxsrc/frontend/apps/impress/src/features/docs/doc-table-content/components/TableContent.tsxsrc/frontend/apps/impress/src/features/docs/doc-table-content/components/TableContentSideBar.tsxsrc/frontend/apps/impress/src/features/docs/doc-table-content/components/index.tssrc/frontend/apps/impress/src/features/docs/docs-grid/components/DocsGrid.tsxsrc/frontend/apps/impress/src/features/header/components/Header.tsxsrc/frontend/apps/impress/src/features/left-panel/components/LeftPanel.tsxsrc/frontend/apps/impress/src/features/left-panel/components/LeftPanelFavoriteItem.tsxsrc/frontend/apps/impress/src/features/left-panel/components/ResizableLeftPanel.tsxsrc/frontend/apps/impress/src/features/left-panel/stores/useLeftPanelStore.tsxsrc/frontend/apps/impress/src/features/right-panel/components/RightPanel.tsxsrc/frontend/apps/impress/src/features/right-panel/components/RightPanelCollapseButton.tsxsrc/frontend/apps/impress/src/features/right-panel/stores/useRightPanelStore.tsxsrc/frontend/apps/impress/src/layouts/MainLayout.tsxsrc/frontend/apps/impress/src/layouts/PageLayout.tsxsrc/frontend/apps/impress/src/layouts/usePanelCoordination.tsxsrc/frontend/apps/impress/src/stores/useResponsiveStore.tsx
💤 Files with no reviewable changes (6)
- src/frontend/apps/impress/src/features/docs/doc-editor/stores/index.ts
- src/frontend/apps/impress/src/features/docs/doc-editor/stores/usePanelEditorStore.tsx
- src/frontend/apps/impress/src/features/docs/doc-table-content/components/TableContent.tsx
- src/frontend/apps/impress/src/features/docs/doc-table-content/components/index.ts
- src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/DocsThreadStore.tsx
- src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/CommentToolbarButton.tsx
AntoLC
force-pushed
the
feat/comment-side-panel
branch
from
f6dc394 to
ca214b0
Compare
AntoLC
force-pushed
the
feat/comment-side-panel
branch
2 times, most recently
from
11866af to
2a5e23a
Compare
AntoLC
added
💬 Design review
Ovgodd
requested changes
May 26, 2026
Ovgodd
left a comment
Ovgodd
left a comment
Collaborator
There was a problem hiding this comment.
Nice feature! I only left a few small suggestions.
From an accessibility perspective, it looks good to me. I’ll ask Sophie to test it once it’s released.
| ); | ||
| }; | ||
|
|
||
| export const CommentSideBarButton = () => { |
Collaborator
There was a problem hiding this comment.
Could we move CommentSideBarButton to a dedicated file?
This would keep CommentSideBar focused on the sidebar content and avoid mixing the panel rendering with the trigger button logic.
Collaborator
Author
There was a problem hiding this comment.
It is by purpose, to understand quickly that this component activate this other component.
| shouldCloseOnInteractOutside={() => true} | ||
| onOpenChange={setOpen} | ||
| > | ||
| <Tooltip content={t('Filter comments')} placement="top"> |
Collaborator
There was a problem hiding this comment.
Little issue with the tooltip that appears under the navbar
| ); | ||
| }; | ||
|
|
||
| export const TableContentSideBarButton = () => { |
Collaborator
There was a problem hiding this comment.
Could we extract TableContentSideBarButton into a dedicated file?
Collaborator
Author
There was a problem hiding this comment.
It is by purpose, to understand quickly that this component activate this other component.
AntoLC
force-pushed
the
feat/comment-side-panel
branch
from
2a5e23a to
4502d7e
Compare
Collaborator
Author
I think there is big accessibility issues, but it should be fixed on Blocknote side I think to be clean. 
|
AntoLC
force-pushed
the
feat/comment-side-panel
branch
from
4502d7e to
26574dc
Compare
Collaborator
Oh nice catch, I didn't see them |
Collaborator
|
Here is the discussion open on blocknote regarding the accessibility problem : TypeCellOS/BlockNote#2791 |
AntoLC
force-pushed
the
feat/comment-side-panel
branch
from
26574dc to
7d4748a
Compare
Add a right panel, it will in a first time be used to display the comment side panel, but it will be used in the future for other features as well.
Add comment side panel to the right panel. We will be able to manage the threads of the document and see the content of the comments in a side panel. The advantage of this approach is that we will be able to: - see the comments that have been removed because of deleted text - see the resolved comments - see the unresolved comments
We move the floating table of content to the right panel. This allows us to have a more consistent UI and to make room for the right sidebar.
Most of the application was switching to mobile view at the medium breakpoint. Medium breakpoint let enough place to display most of the application features and it is more user friendly to switch to mobile view at the small breakpoint. We are switching to mobile view at the small breakpoint to give more place to the application features and to be more user friendly.
On tablet, the place is too tight to have both panels open at the same time. We have a fine-grained control of the panel visibility to display panel depend users interactions and responsive breakpoints.
- improve semantics and aria attributes - gives the focus to the panel when open or switching panels - gives back the focus to the trigger element when closing the panel
AntoLC
force-pushed
the
feat/comment-side-panel
branch
from
7d4748a to
e85e7d6
Compare
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
✨ We are implementing the "comments" inside a right panel.
Listing the comment inside the side panel will give us the ability to:
We moved as well the "table of contents" in the panel, it was conflicting with the "comments" feature.
Demo
Enregistrement.2026-05-22.160322.mp4