[WIKI-320] refactor: page header actions#6946
Conversation
WalkthroughThis update introduces significant refactoring and modularization of page header and editor toolbar components. It removes the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PageHeaderActions
participant PageLockControl
participant PageCopyLinkControl
participant PageFavoriteControl
participant PageArchivedBadge
participant PageOfflineBadge
User->>PageHeaderActions: Interacts with page header
PageHeaderActions->>PageLockControl: Show lock/unlock button
PageHeaderActions->>PageCopyLinkControl: Show copy link button
PageHeaderActions->>PageFavoriteControl: Show favorite toggle
PageHeaderActions->>PageArchivedBadge: Show archived status (if applicable)
PageHeaderActions->>PageOfflineBadge: Show offline status (if applicable)
sequenceDiagram
participant PageRoot
participant PageEditorBody
participant PageEditorToolbarRoot
participant PageCollaboratorsList
PageRoot->>PageEditorBody: Pass editorForwardRef and handleEditorReady
PageRoot->>PageEditorToolbarRoot: Pass page
PageEditorToolbarRoot->>PageCollaboratorsList: Pass page for collaborators display
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
web/ce/components/pages/header/move-control.tsx (1)
1-11:⚠️ Potential issueFix empty object pattern in component props.
The component currently has an empty implementation (returns
null), but there's a syntax issue with the empty object pattern in the props destructuring.Apply this fix:
-export const PageMoveControl = ({}: TPageMoveControlProps) => null; +export const PageMoveControl = (_props: TPageMoveControlProps) => null;Alternatively, if you don't need to reference the props parameter at all:
-export const PageMoveControl = ({}: TPageMoveControlProps) => null; +export const PageMoveControl = (_: TPageMoveControlProps) => null;🧰 Tools
🪛 Biome (1.9.4)
[error] 10-10: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
🧹 Nitpick comments (4)
web/core/hooks/use-page-operations.ts (1)
50-53: Fixed variable casing for consistency.Corrected the variable name from
isfavoriteMenuOpentoisFavoriteMenuOpento maintain consistent camelCase naming convention.web/ce/components/pages/header/move-control.tsx (1)
1-11: Consider adding TODO comment for future implementation.Since this appears to be a stub component that will be implemented in the future, consider adding a TODO comment to indicate the planned functionality.
-export const PageMoveControl = ({}: TPageMoveControlProps) => null; +// TODO: Implement page move functionality +export const PageMoveControl = (_props: TPageMoveControlProps) => null;🧰 Tools
🪛 Biome (1.9.4)
[error] 10-10: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
web/ce/components/pages/header/collaborators-list.tsx (1)
1-10: Consider adding documentation for this placeholder component.This component appears to be a placeholder for displaying page collaborators (as suggested by its name). Since it currently returns
null, it would be helpful to add a comment explaining its intended purpose and when it will be implemented.export type TPageCollaboratorsListProps = { page: TPageInstance; }; +// TODO: Implement collaborators list to display users currently viewing/editing the page export const PageCollaboratorsList = ({ page }: TPageCollaboratorsListProps) => null;🧰 Tools
🪛 Biome (1.9.4)
[error] 10-10: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
web/core/store/pages/base-page.ts (1)
531-536: Remove console.log statementThere's a debug console.log statement that should be removed before merging to production.
setEditorRef = (editorRef: EditorRefApi | null) => { - console.log("store editorRef", editorRef); runInAction(() => { this.editorRef = editorRef; }); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx(0 hunks)web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/header.tsx(3 hunks)web/ce/components/pages/editor/ai/menu.tsx(6 hunks)web/ce/components/pages/header/collaborators-list.tsx(1 hunks)web/ce/components/pages/header/lock-control.tsx(1 hunks)web/ce/components/pages/header/move-control.tsx(1 hunks)web/core/components/pages/dropdowns/edit-information-popover.tsx(0 hunks)web/core/components/pages/dropdowns/index.ts(0 hunks)web/core/components/pages/editor/editor-body.tsx(5 hunks)web/core/components/pages/editor/header/logo-picker.tsx(1 hunks)web/core/components/pages/editor/page-root.tsx(6 hunks)web/core/components/pages/editor/summary/content-browser.tsx(2 hunks)web/core/components/pages/editor/title.tsx(2 hunks)web/core/components/pages/editor/toolbar/extra-options.tsx(0 hunks)web/core/components/pages/editor/toolbar/index.ts(0 hunks)web/core/components/pages/editor/toolbar/info-popover.tsx(5 hunks)web/core/components/pages/editor/toolbar/mobile-root.tsx(0 hunks)web/core/components/pages/editor/toolbar/options-dropdown.tsx(2 hunks)web/core/components/pages/editor/toolbar/root.tsx(1 hunks)web/core/components/pages/editor/toolbar/toolbar.tsx(3 hunks)web/core/components/pages/header/actions.tsx(1 hunks)web/core/components/pages/header/archived-badge.tsx(1 hunks)web/core/components/pages/header/copy-link-control.tsx(1 hunks)web/core/components/pages/header/favorite-control.tsx(1 hunks)web/core/components/pages/header/offline-badge.tsx(1 hunks)web/core/hooks/use-page-operations.ts(3 hunks)web/core/store/pages/base-page.ts(8 hunks)web/core/store/pages/project-page.store.ts(2 hunks)web/styles/globals.css(1 hunks)
💤 Files with no reviewable changes (6)
- web/core/components/pages/dropdowns/index.ts
- web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx
- web/core/components/pages/editor/toolbar/index.ts
- web/core/components/pages/editor/toolbar/mobile-root.tsx
- web/core/components/pages/dropdowns/edit-information-popover.tsx
- web/core/components/pages/editor/toolbar/extra-options.tsx
🧰 Additional context used
🧬 Code Graph Analysis (12)
web/ce/components/pages/header/move-control.tsx (1)
web/core/store/pages/base-page.ts (1)
TPageInstance(67-70)
web/ce/components/pages/header/collaborators-list.tsx (1)
web/core/store/pages/base-page.ts (1)
TPageInstance(67-70)
web/core/components/pages/header/archived-badge.tsx (1)
web/core/store/pages/base-page.ts (1)
TPageInstance(67-70)
web/ce/components/pages/editor/ai/menu.tsx (1)
packages/editor/src/core/types/editor.ts (1)
EditorRefApi(96-113)
web/core/components/pages/header/offline-badge.tsx (2)
web/core/store/pages/base-page.ts (1)
TPageInstance(67-70)packages/ui/src/tooltip/tooltip.tsx (1)
Tooltip(36-110)
web/core/components/pages/header/actions.tsx (6)
web/core/store/pages/base-page.ts (1)
TPageInstance(67-70)web/core/components/pages/header/archived-badge.tsx (1)
PageArchivedBadge(12-21)web/core/components/pages/header/offline-badge.tsx (1)
PageOfflineBadge(13-30)web/ce/components/pages/header/lock-control.tsx (1)
PageLockControl(20-116)web/core/components/pages/editor/toolbar/info-popover.tsx (1)
PageInfoPopover(21-141)web/core/components/pages/header/copy-link-control.tsx (1)
PageCopyLinkControl(12-27)
web/core/components/pages/editor/summary/content-browser.tsx (1)
packages/editor/src/core/extensions/headers.ts (1)
IMarking(4-9)
web/core/components/pages/editor/title.tsx (1)
packages/editor/src/core/types/editor.ts (1)
EditorRefApi(96-113)
web/core/store/pages/project-page.store.ts (1)
web/core/store/pages/project-page.ts (1)
ProjectPage(16-183)
web/core/components/pages/editor/toolbar/toolbar.tsx (1)
web/core/components/pages/editor/toolbar/color-dropdown.tsx (1)
ColorDropdown(22-125)
web/core/components/pages/editor/toolbar/info-popover.tsx (1)
web/core/store/pages/base-page.ts (1)
TPageInstance(67-70)
web/core/components/pages/header/copy-link-control.tsx (2)
web/core/store/pages/base-page.ts (1)
TPageInstance(67-70)web/core/hooks/use-page-operations.ts (1)
usePageOperations(30-209)
🪛 Biome (1.9.4)
web/ce/components/pages/header/move-control.tsx
[error] 10-10: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
web/ce/components/pages/header/collaborators-list.tsx
[error] 10-10: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (52)
web/styles/globals.css (1)
858-944: Well-structured animations for lock/unlock interactions.The keyframe animations and utility classes are clearly defined with appropriate timing and easing functions. The animations will provide clear visual feedback for lock state changes.
web/core/components/pages/editor/header/logo-picker.tsx (1)
26-26: Appropriate transition duration adjustment for smoother animation.Increasing the transition duration from 200ms to 300ms will make the logo picker animation feel more natural and consistent with other animations in the system.
web/core/components/pages/editor/title.tsx (2)
16-16: Simplified editorRef type aligns with broader refactoring.Changing from
React.RefObject<EditorRefApi>toEditorRefApi | nullsimplifies the API and aligns with the new pattern of storing the editor reference directly on the page instance.
56-56: Correctly updated editorRef usage.The optional chaining usage has been properly updated to match the new direct reference type.
web/core/hooks/use-page-operations.ts (2)
150-150: Updated conditional check to use renamed variable.The conditional check now properly uses the corrected variable name
isFavoriteMenuOpen.
202-205: Improved dependency tracking in useMemo.Correctly updated the variable name in the dependency array and added
toggleFavoriteMenuto ensure proper dependency tracking, preventing potential stale closure issues.web/ce/components/pages/editor/ai/menu.tsx (1)
3-3: Refactored editor reference handling, good improvement.The changes consistently update how the editor reference is accessed, shifting from using React's RefObject pattern (
editorRef.current?.method()) to directly accessing methods on the EditorRefApi object (editorRef?.method()). This aligns with the broader refactoring to store the editor reference directly in the page instance rather than passing it through props.Also applies to: 21-21, 76-76, 89-89, 107-107, 126-126
web/core/components/pages/editor/summary/content-browser.tsx (1)
1-1: Good optimization with useCallback.The refactoring of
handleOnClickto useuseCallbackimproves performance by preventing unnecessary re-renders when dependencies don't change. The dependencies array correctly includes botheditorRefandsetSidePeekVisible.Also applies to: 27-33
web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/header.tsx (3)
11-11: Good refactoring for better modularity.The introduction of the
PageHeaderActionscomponent and the constantstoreTypeimproves code organization and reduces duplication.Also applies to: 27-27
35-39: Improved organization of store hook usage.Moving the
usePageStorehook aboveusePageand using the constantstoreTypeimproves readability and maintainability.
114-114: Successfully replaced PageEditInformationPopover with new modular component.The replacement of
PageEditInformationPopoverwithPageHeaderActionsaligns with the PR objective of refactoring the page header components for improved structure and functionality.web/core/components/pages/editor/toolbar/toolbar.tsx (3)
16-18: Props simplified by removing theisHiddenprop.The removal of
isHiddenfrom the Props type aligns with the PR goal of reducing prop drilling and simplifying the component interfaces. This change streamlines the component API.
100-100: Toolbar styling simplified with consistent layout.The toolbar container now has a fixed flex layout with horizontal scrolling and border division, replacing the conditional styling based on
isHidden. This makes the component more predictable and easier to maintain.
134-149: ColorDropdown properly constrained with flex-shrink-0.Wrapping the ColorDropdown in a div with
flex-shrink-0class prevents it from shrinking within the flex container, which is an appropriate CSS practice for maintaining the dropdown's intended size and layout.web/core/components/pages/header/archived-badge.tsx (1)
1-21: Well-structured archived badge component with reactive rendering.This new component is well-designed:
- It uses MobX's
observerfor reactive rendering- It conditionally renders based on
page.archived_at- It uses appropriate icon and formatting for the archived date
- The styling is consistent with the application's design system
The component effectively modularizes the archived status indicator, making it reusable across the application.
web/core/components/pages/header/copy-link-control.tsx (1)
1-27: Clean implementation of page link copy functionality.This component effectively:
- Uses the
usePageOperationshook to access shared functionality- Maintains a consistent UI style with other header controls
- Has a clear, focused responsibility (copying page links)
- Uses MobX's
observerfor reactive renderingThe separation of this functionality into its own component aligns well with the modular design approach of this refactoring.
web/core/components/pages/header/actions.tsx (1)
1-37: Well-structured component for consolidating page header actionsThis new component effectively consolidates various page-related UI elements into a single header action bar. The implementation follows React best practices with proper typing and modular composition.
web/core/components/pages/header/offline-badge.tsx (1)
13-30: Good implementation of conditional rendering for offline stateThe component effectively shows an offline badge only when needed (user is offline and page is editable). The tooltip provides helpful context to users about working offline.
web/core/components/pages/editor/toolbar/options-dropdown.tsx (5)
7-7: Updated import section commentChanged from separate "document editor" and "ui" sections to a single "plane imports" section for better organization.
21-24: Simplified props by removing editorRefSuccessfully removed the
editorRefprop as part of the refactoring to access editor reference directly from the page instance.
27-27: Accessing editorRef from page objectNow destructuring only necessary props, with
editorRefaccessed from page object instead of as a separate prop. This aligns with the PR's goal of reducing prop drilling.
33-33: Using editorRef from page instanceProperly extracting
editorReffrom page instance, which supports the refactoring goal of storing editor reference in the page instance.
124-134: Removed redundant options from dropdown menuRemoved
"copy-link","move", and"toggle-lock"options from the dropdown since they're now handled by dedicated components in the header actions.web/core/components/pages/editor/page-root.tsx (7)
1-1: Well-organized imports with useCallback added.Adding
useCallbackimproves the performance of the component by memoizing the callback function.
43-44: Good simplification of props structure.Removing the
storeTypeprop reduces complexity and aligns with the new approach of centralized editor reference management.
56-57: Effective use of page object destructuring.Destructuring
setEditorReffrom the page object properly leverages the centralized editor reference pattern.
67-75: Well-implemented callback with proper dependency array.Creating a memoized callback with
useCallbackthat handles both setting the editor ready state and the editor reference is a good pattern. The dependency array correctly includespage.editorRefandsetEditorRef.
99-105: Good cleanup on component unmount.Adding an effect to reset the editor reference when the component unmounts prevents memory leaks and ensures proper cleanup.
121-121: Simplified prop passing to PageEditorToolbarRoot.Removing unnecessary props and only passing the
pageobject reduces prop drilling and aligns with the centralized editor reference pattern.
125-128: Consistent prop renaming and handler changes.Renaming
editorReftoeditorForwardRefand updating the handler from directsetEditorReadytohandleEditorReadymaintains consistency with the new architecture.web/core/store/pages/project-page.store.ts (2)
209-221: Improved state preservation with conditional updates.This change enhances how pages are stored by updating existing instances rather than replacing them. This preserves state like editor references between updates, which is crucial for maintaining UI consistency.
- if (page?.id) set(this.data, [page.id], new ProjectPage(this.store, page)); + for (const page of pages) { + if (page?.id) { + const existingPage = this.getPageById(page.id); + if (existingPage) { + // If page already exists, update all fields except name + const { name, ...otherFields } = page; + existingPage.mutateProperties(otherFields, false); + } else { + // If new page, create a new instance with all data + set(this.data, [page.id], new ProjectPage(this.store, page)); + } + } + }
253-262: Consistent update pattern in fetchPageDetails.The same pattern of checking for existing pages before creating new ones is applied here, which ensures a consistent approach throughout the store and maintains editor references.
web/core/components/pages/editor/editor-body.tsx (4)
47-50: Clear prop type definitions aligned with new architecture.Renaming
editorReftoeditorForwardRefand simplifying thehandleEditorReadytype signature from a state dispatcher to a callback function improves clarity and aligns with the new architecture.
73-74: Effective destructuring of page properties.Now retrieving
editorRefdirectly from thepageobject aligns with the centralized editor reference pattern and reduces prop drilling.
175-179: Simplified editor reference passing to child components.Directly passing
editorReffrom the page object toPageContentBrowsercomponents streamlines the code and maintains consistency.
199-200: Consistent ref forwarding to CollaborativeDocumentEditorWithRef.Using
editorForwardReffor therefprop maintains consistency with the new naming convention throughout the component.web/core/components/pages/editor/toolbar/info-popover.tsx (5)
2-2: Added MobX observer for reactive rendering.Wrapping the component with
observerensures it properly reacts to changes in the MobX store, which is important for real-time updates.
21-23: Simplified component by removing unnecessary props.The component now only requires the
pageprop and accesseseditorRefdirectly from it, which aligns with the centralized editor reference pattern.
40-41: Direct access to editor reference from page object.Accessing
editorRefdirectly from thepageobject instead of through props follows the new centralized pattern consistently.
72-81: Improved container and button styling.The updated container and button styles with better utility classes improve the visual consistency and user experience.
114-115: Enhanced date display with relative time format.Changing from
renderFormattedDatetocalculateTimeAgoShortwith " ago" provides a more intuitive representation of when the page was last edited.web/core/components/pages/editor/toolbar/root.tsx (3)
18-20: Clean and simplified prop destructuringThe code now directly accesses
editorReffrom thepageprop, which reduces prop drilling and simplifies the component API.
27-31: Improved animation transition handlingThe toolbar container now uses proper CSS transitions with conditional classes to animate its height and overflow, creating a smoother user experience when showing/hiding the toolbar.
41-44: Better component composition and simplified conditional renderingThe component now:
- Conditionally renders the
PageToolbaronly wheneditorRefexists- Adds the new
PageCollaboratorsListcomponent alongside the toolbar- Removes the need for checking
editorReadyflagThis approach is cleaner and more maintainable.
web/core/store/pages/base-page.ts (4)
15-15: Good addition of editorRef to the page modelAdding the
editorRefproperty to theTBasePageinterface along with the corresponding methods is a good approach for centralizing editor reference management.Also applies to: 37-38
75-75: Proper Observable implementation for editorRefThe
editorRefproperty is correctly implemented as an observable withobservable.refwhich is appropriate for references to complex objects like the editor API.Also applies to: 133-133
437-464: Improved error handling for updatePageLogoThe refactored
updatePageLogomethod now:
- Properly saves the original logo props before updating
- Uses a try/catch block for robust error handling
- Correctly restores the original state if an error occurs
This makes the method more resilient to failures.
519-529: Well-implemented mutateProperties methodThe new
mutatePropertiesmethod allows batch updates to page properties with the option to exclude the name property. This is a clean approach to updating multiple properties at once.web/ce/components/pages/header/lock-control.tsx (4)
13-14: Clear type definition for lock statesThe
LockDisplayStatetype with three distinct states helps maintain clarity throughout the component implementation.
20-34: Well-structured component initializationThe component properly:
- Initializes state based on current lock status
- Uses refs for timing operations
- Accesses necessary operations via custom hooks
- Uses MobX observer pattern for reactivity
This approach ensures clean separation of concerns and reactive updates.
37-69: Robust effect management for state transitionsThe component uses
useEffectto:
- Clean up timers on unmount to prevent memory leaks
- Handle state transitions with appropriate timing
- Track previous state via refs to determine transition direction
This implementation ensures smooth transitions between lock states with proper cleanup.
73-114: Well-implemented conditional rendering with animation statesThe component renders different UI elements based on the current display state with:
- Appropriate tooltips for the neutral state
- Animation classes for visual transitions
- Proper aria-labels for accessibility
- Clean transitions between states
The animations create a polished user experience while maintaining accessibility.
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
* refactor: page header actions * chore: update toolbar component * chore: update archived and lock badge colors * chore: added observer to favorite control
Description
This PR updates the page header actions.
Other improvements-
editorRefis now saved in page instance instead of prop drilling.Type of Change
Media
Screen.Recording.2025-04-17.at.20.30.28.mov
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Removals