♻️(frontend) refacto of the grid documents - #2534
Open
AntoLC wants to merge 3 commits into
Open
Conversation
Contributor
|
Size Change: +2.39 kB (+0.05%) Total Size: 4.36 MB 📦 View Changed
|
AntoLC
force-pushed
the
refacto/grid-document-v3
branch
5 times, most recently
from
July 23, 2026 14:05
9de768e to
f3c05a8
Compare
Collaborator
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
WalkthroughThe pull request refactors the document grid with target-aware queries, sorting controls, responsive layouts, updated document actions, and hook-based drag-and-drop. End-to-end tests now cover sorting and revised grid interactions. Shared document icons, translations, loading behavior, page styling, focus-visible states, and related changelog entries were also updated. Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 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 `@CHANGELOG.md`:
- Line 17: Update the frontend grid documents changelog entry to replace the
typo “refacto” with “refactor” or “refactoring,” preserving the rest of the
entry unchanged.
In `@src/frontend/apps/e2e/__tests__/app-impress/doc-grid.spec.ts`:
- Around line 391-401: Strengthen the sorting assertions in the doc-grid test by
verifying the relative DOM positions of the filtered rows, not only their
counts. After clicking “Sort by Name,” assert docA appears before docB; after
clicking “Sorted by Name,” assert docZ is the first matching row. Keep the
existing visibility/count checks for excluded documents.
In
`@src/frontend/apps/impress/src/features/docs/doc-management/components/SimpleDocItem.tsx`:
- Around line 62-115: Refactor the icon rendering in the component’s main
ternary so the shared DocsIcon/SubdocsIcon selection is computed once, with
isChild selecting SubdocsIcon and the default selecting DocsIcon. Render one
Icon instance, conditionally wrap it in the pinned relative Box only when
isPinned, and keep the pinned data-testid and overlay PinnedIcon limited to that
wrapper.
In
`@src/frontend/apps/impress/src/features/docs/docs-grid/components/DocGridContentList.tsx`:
- Around line 300-320: Update DocsGridItem so the useDraggable boxProps,
including role and tabIndex from attributes, are applied after its hardcoded
props and are not overridden; preserve the existing draggable listeners and
attributes wiring so the item remains keyboard-focusable.
- Around line 117-119: Guard the document.body access in the dndAccessibility
setup within the DocGridContentList component so it is only evaluated after
client mounting. Avoid creating the accessibility object with document.body
during server rendering or when isLargeScreen is false, and pass the guarded
value to DndContext while preserving the existing drag behavior.
In
`@src/frontend/apps/impress/src/features/docs/docs-grid/components/DocsGrid.tsx`:
- Around line 119-138: The role-less grid Box in DocsGrid.tsx should no longer
carry aria-label; move the translated “Documents grid” label to the nested Box
with role="list". In SimpleDocItem.tsx, remove aria-label={itemAriaLabel} from
the plain container, relying on DocsGridItem’s enclosing link/listitem for the
accessible name.
- Around line 144-162: Remove the redundant !isFetching && hasNextPage
conditional inside the InView component in DocsGrid. Render the More docs Button
directly within InView, preserving its existing click handler, styling,
translation, and outer hasNextPage && !loading guard.
In
`@src/frontend/apps/impress/src/features/docs/docs-grid/components/DocsGridActions.tsx`:
- Around line 244-266: Move the doc.abilities.restore guard to the start of the
component logic, before constructing the options array in DocsGridActions.
Return null immediately when restore is unavailable, while preserving the
existing restore callback and options behavior for allowed documents.
- Around line 222-242: Update the useRestoreDoc configuration’s onError callback
to accept the error argument supplied by React Query and interpolate that
argument in the restore failure toast, instead of reading the hook-level error
binding. Leave the existing success behavior and toast options unchanged.
In
`@src/frontend/apps/impress/src/features/docs/docs-grid/components/DocsGridColumnName.tsx`:
- Around line 28-32: Update the header controls using toggleOrdering so each
Button’s label includes the visible column text and sort icon, making the entire
header cell clickable. Remove the separate text outside the Button and reuse the
combined label for accessibility instead of maintaining duplicated label and
ariaLabel values.
- Around line 113-129: Update the sort button in DocsGridColumnName to use
distinct neutral, ascending, and descending glyphs based on isActive and isDesc,
while retaining the existing color state. Ensure inactive columns use the
neutral icon, active ascending columns use an up icon, and active descending
columns use a down icon.
In
`@src/frontend/apps/impress/src/features/docs/docs-grid/components/DocsGridItem.tsx`:
- Around line 81-95: Remove tabIndex={0} from the inner Box in DocsGridItem so
the anonymous container is not included in keyboard navigation. Keep keyboard
focus and activation behavior on the existing StyledLink, where handleKeyDown is
defined.
- Around line 92-95: Remove the duplicate grid-template-columns declaration in
the CSS template for the DocsGridItem component, keeping a single
grid-template-columns: subgrid rule alongside the existing grid-column
declaration.
🪄 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 Plus
Run ID: 2ebbe7cf-d552-4fcf-bffe-fe4b9e139955
⛔ Files ignored due to path filters (6)
src/frontend/apps/impress/src/assets/icons/ui-kit/Docs.svgis excluded by!**/*.svgsrc/frontend/apps/impress/src/assets/icons/ui-kit/Subdocs.svgis excluded by!**/*.svgsrc/frontend/apps/impress/src/assets/icons/ui-kit/arrow-up-down.svgis excluded by!**/*.svgsrc/frontend/apps/impress/src/assets/icons/ui-kit/bin.svgis excluded by!**/*.svgsrc/frontend/apps/impress/src/assets/icons/ui-kit/clock.svgis excluded by!**/*.svgsrc/frontend/apps/impress/src/assets/icons/ui-kit/pinned.svgis excluded by!**/*.svg
📒 Files selected for processing (29)
CHANGELOG.mdsrc/frontend/apps/e2e/__tests__/app-impress/doc-create.spec.tssrc/frontend/apps/e2e/__tests__/app-impress/doc-grid-move.spec.tssrc/frontend/apps/e2e/__tests__/app-impress/doc-grid.spec.tssrc/frontend/apps/e2e/__tests__/app-impress/doc-member-create.spec.tssrc/frontend/apps/e2e/__tests__/app-impress/utils-common.tssrc/frontend/apps/e2e/__tests__/app-impress/utils-signin.tssrc/frontend/apps/impress/src/components/ErrorPage.tsxsrc/frontend/apps/impress/src/components/Loading.tsxsrc/frontend/apps/impress/src/features/docs/doc-header/components/AlertRestore.tsxsrc/frontend/apps/impress/src/features/docs/doc-management/components/SimpleDocItem.tsxsrc/frontend/apps/impress/src/features/docs/docs-grid/api/useDocsGridQuery.tsxsrc/frontend/apps/impress/src/features/docs/docs-grid/components/DocGridContentList.tsxsrc/frontend/apps/impress/src/features/docs/docs-grid/components/DocMoveModal.tsxsrc/frontend/apps/impress/src/features/docs/docs-grid/components/DocsGrid.tsxsrc/frontend/apps/impress/src/features/docs/docs-grid/components/DocsGridActions.tsxsrc/frontend/apps/impress/src/features/docs/docs-grid/components/DocsGridColumnName.tsxsrc/frontend/apps/impress/src/features/docs/docs-grid/components/DocsGridItem.tsxsrc/frontend/apps/impress/src/features/docs/docs-grid/components/DocsGridLoader.tsxsrc/frontend/apps/impress/src/features/docs/docs-grid/components/DocsGridTrashbinActions.tsxsrc/frontend/apps/impress/src/features/docs/docs-grid/components/Draggable.tsxsrc/frontend/apps/impress/src/features/docs/docs-grid/components/Droppable.tsxsrc/frontend/apps/impress/src/features/docs/docs-grid/components/__tests__/DocsGridItemDate.test.tsxsrc/frontend/apps/impress/src/features/docs/docs-grid/components/index.tssrc/frontend/apps/impress/src/features/docs/docs-grid/hooks/useResponsiveDocGrid.tsxsrc/frontend/apps/impress/src/features/left-panel/components/LeftPanel.tsxsrc/frontend/apps/impress/src/pages/404.tsxsrc/frontend/apps/impress/src/pages/docs/index.tsxsrc/frontend/apps/impress/src/pages/globals.css
💤 Files with no reviewable changes (8)
- src/frontend/apps/impress/src/features/docs/docs-grid/components/Droppable.tsx
- src/frontend/apps/impress/src/features/docs/docs-grid/hooks/useResponsiveDocGrid.tsx
- src/frontend/apps/impress/src/features/docs/docs-grid/components/index.ts
- src/frontend/apps/impress/src/features/docs/docs-grid/components/Draggable.tsx
- src/frontend/apps/impress/src/features/docs/docs-grid/components/DocMoveModal.tsx
- src/frontend/apps/impress/src/features/docs/docs-grid/components/DocsGridLoader.tsx
- src/frontend/apps/impress/src/features/docs/docs-grid/components/DocsGridTrashbinActions.tsx
- src/frontend/apps/impress/src/features/docs/docs-grid/components/tests/DocsGridItemDate.test.tsx
Comment on lines
+28
to
+32
| const toggleOrdering = (field: 'title' | 'updated_at') => { | ||
| setOrdering((prevOrdering) => | ||
| prevOrdering === field ? (`-${field}` as DocsOrdering) : field, | ||
| ); | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Make the whole header cell the sort control.
The visible Name / Last modified text sits outside the Button, so the only pointer target is the nano icon. Wrapping the label inside the button (it already accepts ReactNode via label) would give a much larger hit area and remove the duplicated label/ariaLabel pair.
Also applies to: 43-57
🤖 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/impress/src/features/docs/docs-grid/components/DocsGridColumnName.tsx`
around lines 28 - 32, Update the header controls using toggleOrdering so each
Button’s label includes the visible column text and sort icon, making the entire
header cell clickable. Remove the separate text outside the Button and reuse the
combined label for accessibility instead of maintaining duplicated label and
ariaLabel values.
Collaborator
Author
There was a problem hiding this comment.
Acceptable for the moment, we will wait for the design review.
There was a problem hiding this comment.
@AntoLC, understood. We can revisit the header hit area and accessibility treatment once the design review provides direction.
We have refactored the document grid feature to fit the new design system and to improve the code quality.
Lot of elements did not have a focus style, or the focus style was not consistent. We now have a global focus style that is applied to all elements that are focusable. The style is still overridable for specific elements, so buttons and other elements still have their own focus style. We will have probably to improve the focus style in the future of some elements, but this is a good start.
We can now sort the documents list by clicking on the column name. The sorting is done in ascending or descending order.
AntoLC
force-pushed
the
refacto/grid-document-v3
branch
from
July 29, 2026 08:54
f3c05a8 to
0e6f2f7
Compare
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
1 participant
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 have refactored the document grid feature to fit the new design system and to improve the code quality.
We can now sort the data by name or updating date (asc / desc).
Ref #2364
Demo
Enregistrement.2026-07-27.153020.mp4