-
Notifications
You must be signed in to change notification settings - Fork 3.4k
style: page editor width and layout updates #6826
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
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces multiple enhancements across the editor and page components. Key changes involve the adoption of a utility function for class name management and conditional application of layout classes based on the display configuration. Several components have been updated to accept additional styling props while redundant props and components have been removed. Minor adjustments in error handling and CSS rules also occur. Overall, the modifications update styling, layout control, and type definitions to support a new wide layout configuration without altering core functionality. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🪧 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/editor/src/styles/editor.css (1)
11-11: Consider alternatives to!importantWhile the change ensures the outline property takes precedence, using
!importantis generally discouraged in CSS as it breaks the natural cascading of styles and can make future maintenance harder.Consider increasing specificity through more targeted selectors rather than using
!importantif possible.packages/editor/src/styles/variables.css (1)
188-193: Container Query for Wide Header Layout (Mid-Range Widths)
The media query using@container page-header-container (min-width: 912px) and (max-width: 1344px)applies a static padding withvar(--wide-content-margin)(96px) via an!importantrule. While this enforces layout consistency, consider if the use of!importantmight restrict future styling overrides.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
packages/editor/src/core/components/editors/document/collaborative-editor.tsx(2 hunks)packages/editor/src/core/components/editors/document/loader.tsx(1 hunks)packages/editor/src/core/components/editors/document/page-renderer.tsx(1 hunks)packages/editor/src/core/components/editors/document/read-only-editor.tsx(2 hunks)packages/editor/src/core/components/editors/editor-container.tsx(1 hunks)packages/editor/src/core/constants/config.ts(1 hunks)packages/editor/src/core/helpers/common.ts(1 hunks)packages/editor/src/core/types/config.ts(1 hunks)packages/editor/src/styles/editor.css(1 hunks)packages/editor/src/styles/variables.css(3 hunks)web/ce/components/pages/editor/embed/issue-embed-upgrade-card.tsx(1 hunks)web/core/components/pages/editor/editor-body.tsx(4 hunks)web/core/components/pages/editor/header/extra-options.tsx(1 hunks)web/core/components/pages/editor/header/mobile-root.tsx(2 hunks)web/core/components/pages/editor/header/root.tsx(3 hunks)web/core/components/pages/editor/page-root.tsx(1 hunks)web/core/components/pages/editor/summary/content-browser.tsx(2 hunks)web/core/components/pages/editor/summary/heading-components.tsx(1 hunks)web/core/components/pages/editor/summary/index.ts(0 hunks)web/core/components/pages/editor/summary/popover.tsx(0 hunks)web/core/components/pages/editor/title.tsx(4 hunks)web/core/components/pages/loaders/page-content-loader.tsx(2 hunks)web/core/components/pages/version/editor.tsx(2 hunks)
💤 Files with no reviewable changes (2)
- web/core/components/pages/editor/summary/index.ts
- web/core/components/pages/editor/summary/popover.tsx
🧰 Additional context used
🧬 Code Definitions (3)
web/core/components/pages/editor/page-root.tsx (1)
web/core/components/pages/editor/header/root.tsx (1)
PageEditorHeaderRoot(21-56)
web/core/components/pages/editor/editor-body.tsx (3)
web/core/components/pages/loaders/page-content-loader.tsx (1)
PageContentLoader(11-85)web/core/components/pages/editor/summary/content-browser.tsx (1)
PageContentBrowser(13-65)web/core/components/pages/editor/title.tsx (1)
PageEditorTitle(23-89)
web/core/components/pages/editor/header/root.tsx (3)
web/core/store/pages/project-page.ts (1)
isContentEditable(167-177)web/core/components/pages/editor/header/extra-options.tsx (1)
PageExtraOptions(30-103)web/core/components/pages/editor/header/mobile-root.tsx (1)
PageEditorMobileHeaderRoot(17-32)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (59)
packages/editor/src/core/types/config.ts (1)
32-32: LGTM: Added optionalwideLayoutpropertyThe addition of the optional
wideLayoutboolean property to theTDisplayConfigtype is clean and consistent with the existing type structure. This change properly supports the PR objective of improving page layout and width configuration.packages/editor/src/core/constants/config.ts (1)
8-8: LGTM: Default value forwideLayoutis appropriateSetting the default value to
falsefor the newwideLayoutproperty is a good choice as it maintains backward compatibility with existing implementations while allowing opt-in for the enhanced layout.packages/editor/src/core/components/editors/editor-container.tsx (1)
77-77: LGTM: Conditional class implementation for wide layoutThe conditional application of the
"wide-layout"class using thecnutility function is clean and effective. This approach properly integrates the new layout configuration option with the component's styling.packages/editor/src/core/helpers/common.ts (1)
52-52: Clean code improvement: Simplified catch blockGood cleanup by removing the unused error variable from the catch block. This makes the code more concise while maintaining the same functionality.
packages/editor/src/core/components/editors/document/read-only-editor.tsx (2)
4-4: Added utility for class name managementGood addition of the
cnutility import from@plane/utils. This will help with more flexible and maintainable class name handling.
84-84: Enhanced styling with document-editor classThe addition of the "document-editor" class to the editor container aligns with the PR objective of improving the editor width and layout. Using the
cnutility creates a clean way to combine existing classes with the new one.packages/editor/src/core/components/editors/document/collaborative-editor.tsx (4)
4-4: Added utility for class name managementGood addition of the
cnutility import from@plane/utils. This will help with more flexible and maintainable class name handling.
78-80: Improved editor width configurationThis change directly implements the PR objective of adjusting editor width for better readability in both normal and full-width modes. The implementation uses:
- A default max width of 720px
- An increased max width of 1152px when wide layout is enabled
- Smooth transitions between states
This is a well-structured approach that will enhance the user experience.
82-82: Applied consistent styling to loader componentGood enhancement to ensure the DocumentContentLoader maintains the same width constraints as the main editor. This creates a more consistent visual experience during loading states.
90-90: Enhanced styling with document-editor classThe addition of the "document-editor" class to the editor container aligns with the PR objective of improving the editor width and layout. Using the
cnutility creates a clean way to combine existing classes with the new one.web/core/components/pages/editor/header/extra-options.tsx (1)
72-72: Layout adjustment: Removed justify-end classRemoving the
justify-endclass changes how the page options are positioned within their container. This appears to be part of the broader layout improvements mentioned in the PR objectives.Please verify that this alignment change doesn't negatively impact the header's appearance, particularly on smaller screen sizes where space might be more constrained.
packages/editor/src/core/components/editors/document/page-renderer.tsx (1)
135-135: Improved spacing with removal of negative margin.Removing the
-mx-5class eliminates the negative horizontal margins, which aligns with the PR objective of enhancing whitespace around the edges for better readability. This change maintains the component's width withw-fullwhile ensuring proper spacing.web/core/components/pages/editor/page-root.tsx (1)
106-106: Clean simplification of component props.The removal of the
sidePeekVisibleprop simplifies the component's interface and aligns with the broader changes to streamline the UI across related components. This change contributes to the more minimalist design approach mentioned in the PR objectives.web/core/components/pages/editor/summary/content-browser.tsx (3)
10-10: Good addition of optional prop for enhanced customization.Adding the optional
showOutlineprop with proper TypeScript typing enhances component flexibility while maintaining backward compatibility.
14-14: Well-implemented prop with appropriate default value.The destructuring with default value
falseensures backward compatibility for existing usages of the component.
41-62: Effective implementation of minimalist outline view.The conditional rendering based on the
showOutlineprop successfully implements the PR objective of revamping the Table of Contents with a minimalist design. The varying widths based on heading levels create a clear visual hierarchy, and the component maintains consistent key generation across both rendering paths.packages/editor/src/core/components/editors/document/loader.tsx (4)
1-3: Well-structured imports with utility function for class management.Adding the
cnutility import from@plane/utilsis a good practice for managing conditional class names.
5-7: Good TypeScript typing for enhanced component flexibility.Adding a properly typed interface with an optional
classNameprop follows best practices and improves component reusability.
9-11: Clean component signature with proper props destructuring.The updated component signature and props destructuring make the component more maintainable and easier to understand.
13-13: Flexible styling with proper class composition.Using the
cnutility to combine the base class with any providedclassNameenhances styling flexibility. The removal of fixed padding (px-5) in favor ofsize-fullallows parent components to control the spacing, supporting the PR's layout improvement objectives.web/core/components/pages/editor/title.tsx (5)
20-20: Good addition of the widthClassName propAdding this property to control width styling is a clean approach that supports the PR's goal of improving editor readability in different width modes.
30-33: Improved naming and class structureRenaming to
titleFontClassNamebetter represents its purpose and removingbg-transparenthelps separate styling concerns.
36-36: Enhanced container stylingThe updated container class with
py-3andpage-title-containerimproves spacing and makes selector targeting more semantic.
41-41: Good use of width controlApplying the
widthClassNameto the read-only title ensures consistent width control across different states.
51-85: Improved structure for editable titleWrapping the TextArea in a div with width control creates better structural consistency between read-only and editable states, which improves maintainability.
web/ce/components/pages/editor/embed/issue-embed-upgrade-card.tsx (2)
11-11: Improved responsive layoutThe updated class list with flex layout properties and responsive utilities (
max-md:flex-wrap) enhances the card's appearance across different screen sizes.
17-22: Simplified structure and improved terminologyThe streamlined structure with proper flex alignment and the text change from "work items" to "issues" improves clarity and consistency in terminology throughout the application.
web/core/components/pages/version/editor.tsx (2)
11-11: Clean import organizationMoving
useMemberfrom comments to active imports improves code organization while maintaining the functionality.
49-49: Support for wide layoutSetting
wideLayout: truein the display configuration directly supports the PR's goal of improving readability in full-width mode.web/core/components/pages/editor/header/root.tsx (4)
4-4: Simplified importsThe updated imports line maintains necessary component references while removing unused ones.
22-22: Removed unnecessary propsRemoving the sidePeek-related props simplifies the component API, making it easier to maintain and understand.
33-50: Improved header structure and responsivenessThe new
page-header-containerstructure with conditional styling based onisFullWidthprovides better support for different layout modes. The flex layout with proper justification improves the component's visual balance.
52-52: Simplified mobile header propsThe component now passes only the necessary props to the mobile header component, improving the clarity of the parent-child relationship.
web/core/components/pages/editor/header/mobile-root.tsx (1)
5-5: Code simplification and prop cleanup.The component has been simplified by removing the
sidePeekVisibleandsetSidePeekVisibleprops, which aligns with the broader refactoring efforts across the application. These props were likely used for controlling the visibility of the page summary popover, which has been removed from this component.Also applies to: 11-15, 18-18
web/core/components/pages/editor/editor-body.tsx (7)
14-15: Good use of utility imports.The updated imports bring in the
cnutility from@plane/utilsfor class name management andERowVariantfor layout control, which improves the component's flexibility.
91-93: Enhanced display configuration with isFullWidth.Adding
wideLayout: isFullWidthto the display configuration properly ties the editor's layout to the application's state. Make sure to includeisFullWidthin the dependency array to ensure the memo is properly updated when the width changes.
153-158: Well-structured class management with the cn utility.Good use of the
cnutility to create a responsive block width class that adapts based on theisFullWidthstate. The max-width values (720px for normal, 1152px for wide) provide appropriate reading widths for different screen sizes.
160-160: Improved loader with dynamic width class.Passing the
blockWidthClassNameto thePageContentLoaderensures consistent width behavior between the loading state and the loaded editor.
163-180: Elegant Table of Contents implementation.The new Table of Contents implementation is well-structured with:
- A minimalist outline view by default
- An expanded view on hover with full content
- Proper scrolling behavior with
max-h-[70vh]and scrollbar styling- Smooth transitions with
transition-all duration-300This approach improves the user experience by providing quick navigation without occupying too much screen space.
181-187: Proper title styling with consistent width.Passing the
blockWidthClassNameto thePageEditorTitlecomponent ensures the title follows the same width constraints as the content, maintaining a consistent layout.
188-217: Well-structured editor with proper layout configuration.The
CollaborativeDocumentEditorWithRefimplementation is well organized with all necessary props for functionality and styling. The component now properly receives thedisplayConfigwith thewideLayoutproperty to control its width.web/core/components/pages/loaders/page-content-loader.tsx (3)
3-9: Improved component flexibility with props type.Adding a
Propstype with an optionalclassNameprop improves the component's flexibility and type safety. Thecnutility import will help with class name management.
11-15: Proper class composition with the cn utility.The component now properly accepts and applies the
classNameprop using thecnutility, allowing parent components to customize its appearance while maintaining default styles.
41-82: Modernized layout with improved structure.The content layout has been updated with:
- Modern Tailwind utilities like
size-fullinstead of explicit width/height- Proper padding with
pt-[64px]to match the editor layout- Improved nesting structure for loader items
- Consistent spacing and alignment
These changes improve the visual fidelity of the loading state to match the actual editor.
web/core/components/pages/editor/summary/heading-components.tsx (2)
1-4: Improved type declarations with proper exports.Using
import typeforIMarkingand exporting the renamedTHeadingComponentPropsimproves type safety and allows other modules to reuse these types.
9-17: Enhanced heading buttons with smooth transitions.Adding the
transition-colorsclass to all heading buttons provides a smoother hover experience when users interact with the Table of Contents. This is a subtle but valuable UX improvement.Also applies to: 19-27, 29-37
packages/editor/src/styles/variables.css (13)
1-19: Clean Declaration of Global CSS Variables
The:rootblock clearly defines text colors and layout dimensions (such as normal and wide content widths/margins). This will help maintain a consistent design system across the application.
21-31: Well-Scoped Theme Background Colors
The background color variables for both light and dark themes are defined in dedicated blocks. The values appear cohesive; however, please verify that these colors meet the required accessibility contrast ratios.
44-47: Consistent Font Size and Placeholder Styling
The introduction of the--color-placeholdervariable in the.editor-containeris a nice touch to keep placeholder text styling flexible. Ensure that--color-text-100is defined elsewhere so that the fallback is reliable.
48-90: Responsive Font Sizing and Line Heights Configurations
The two modifier classes for font sizes and line heights (.large-fontand.small-font) are thorough and allow for flexible typography scaling. No adjustments needed here unless future design tweaks require further refinements.
94-100: Default Font Style Variable Set
The new declaration of--font-style: "Inter", sans-serif;reinforces a consistent visual identity across the editor. Just ensure that the "Inter" font is loaded or provided as a fallback where needed.
151-152: Clear Section Delimitation
The comment marking the end of the font size and style section keeps the CSS organized.
153-167: Robust Header Container Layout Configuration
The container query for#page-header-containerleverages custom properties and the new container naming strategy. The dynamic adjustment of--header-widthbased on the layout mode and calculated padding (usingcalc(((100% - var(--header-width)) / 2) - 10px)) is well thought out.
169-172: Straightforward Content Container Setup
The configuration for#page-content-containeris succinct and effectively sets the container’s name and type.
174-186: Adaptive Document Editor Width
The.editor-container.document-editorblock correctly sets the--editor-content-widthfor normal and wide layouts and applies it to the.ProseMirrorelement. This ensures that the main content is centered and respects the design’s responsive constraints.
203-208: Appropriate Padding for Small Header Containers
For containers with a maximum width of 760px, usingvar(--normal-content-margin)ensures a consistent 20px padding, as expected from the comment.
211-218: Balanced Content Container Padding for Mid-Sized Screens
The container query for#page-content-containerfor screens between 912px and 1344px correctly applies the wide layout margins, ensuring that the content remains proportionate on larger displays.
220-227: Responsive Adjustments for Smaller Content Containers
For container widths below 912px, the use ofvar(--normal-content-margin)for both left and right paddings yields a balanced layout, complementing the overall design approach.
229-240: Optimized Layout for Narrow Normal Mode
The final container query for widths below 760px not only standardizes padding usingvar(--normal-content-margin)but also hides the.page-summary-container, which is a considerate design adjustment for limited screen space.
Description
This PR includes various improvements on Pages for a better user experience-
Type of Change
Screenshots and Media (if applicable)
Screen.Recording.2025-03-26.at.19.30.10.mov
Screen.Recording.2025-03-26.at.19.59.49.mov
Summary by CodeRabbit
New Features
Refactor
Style
Chores