Skip to content

[WIKI-319] chore: remove bottom border when toolbar is hidden #6943

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.

Sign up for GitHub

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

Merged
sriramveeraghanta merged 2 commits into from
Apr 17, 2025
Merged

[WIKI-319] chore: remove bottom border when toolbar is hidden #6943

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f7428af
chore: remove border when toolbar is hidden
aaryan610 Apr 17, 2025
d5fc7ff
chore: add stricter conditions
aaryan610 Apr 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions web/core/components/pages/editor/toolbar/mobile-root.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import { observer } from "mobx-react";
// plane imports
import { EditorRefApi } from "@plane/editor";
// components
import { Header, EHeaderVariant } from "@plane/ui";
// components
import { PageExtraOptions, PageToolbar } from "@/components/pages";
// hooks
import { usePageFilters } from "@/hooks/use-page-filters";
// plane web hooks
import { EPageStoreType } from "@/plane-web/hooks/store";
// store
Expand All @@ -18,14 +21,16 @@ export const PageEditorMobileHeaderRoot: React.FC<Props> = observer((props) => {
const { editorRef, page, storeType } = props;
// derived values
const { isContentEditable } = page;
// page filters
const { isStickyToolbarEnabled } = usePageFilters();

return (
<>
<Header variant={EHeaderVariant.SECONDARY}>
<PageExtraOptions editorRef={editorRef} page={page} storeType={storeType} />
</Header>
<Header variant={EHeaderVariant.TERNARY}>
{isContentEditable && editorRef && <PageToolbar editorRef={editorRef} />}
{isContentEditable && editorRef && <PageToolbar editorRef={editorRef} isHidden={!isStickyToolbarEnabled} />}
</Header>
</>
);
Expand Down
8 changes: 5 additions & 3 deletions web/core/components/pages/editor/toolbar/root.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,25 @@ export const PageEditorToolbarRoot: React.FC<Props> = observer((props) => {
const { isFullWidth, isStickyToolbarEnabled } = usePageFilters();
// derived values
const resolvedEditorRef = editorRef.current;
const shouldHideToolbar = !isStickyToolbarEnabled || !isContentEditable;

if (!resolvedEditorRef) return null;

return (
<div id="page-toolbar-container">
<div
className={cn(
"hidden md:flex items-center relative min-h-[52px] page-toolbar-content border-b border-custom-border-200 px-page-x transition-all duration-200 ease-in-out",
"hidden md:flex items-center relative min-h-[52px] page-toolbar-content px-page-x border-b border-transparent transition-all duration-200 ease-in-out",
{
"wide-layout": isFullWidth,
"border-custom-border-200": true,
}
Comment on lines +37 to 41
Copy link
Contributor

@coderabbitai coderabbitai bot Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Issue with border styling logic

The border is set to transparent by default but then border-custom-border-200 is applied unconditionally with "border-custom-border-200": true. This means the border will always be visible regardless of whether the toolbar is hidden or not, which contradicts the PR objective of removing the border when the toolbar is hidden.

The border visibility should be conditional on the toolbar visibility state. Apply this fix:

"hidden md:flex items-center relative min-h-[52px] page-toolbar-content px-page-x border-b border-transparent transition-all duration-200 ease-in-out",
{
  "wide-layout": isFullWidth,
-  "border-custom-border-200": true,
+  "border-custom-border-200": !shouldHideToolbar,
}
📝 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.

Suggested change
"hidden md:flex items-center relative min-h-[52px] page-toolbar-content px-page-x border-b border-transparent transition-all duration-200 ease-in-out",
{
"wide-layout": isFullWidth,
"border-custom-border-200": true,
}
"hidden md:flex items-center relative min-h-[52px] page-toolbar-content px-page-x border-b border-transparent transition-all duration-200 ease-in-out",
{
"wide-layout": isFullWidth,
"border-custom-border-200": !shouldHideToolbar,
}

)}
>
<div className="max-w-full w-full flex items-center justify-between">
<div>
{isStickyToolbarEnabled && editorReady && isContentEditable && editorRef.current && (
<PageToolbar editorRef={editorRef?.current} />
{editorReady && resolvedEditorRef && (
<PageToolbar editorRef={resolvedEditorRef} isHidden={shouldHideToolbar} />
)}
</div>
<PageExtraOptions editorRef={resolvedEditorRef} page={page} storeType={storeType} />
Expand Down
13 changes: 11 additions & 2 deletions web/core/components/pages/editor/toolbar/toolbar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { cn } from "@/helpers/common.helper";

type Props = {
editorRef: EditorRefApi;
isHidden: boolean;
};

type ToolbarButtonProps = {
Expand Down Expand Up @@ -63,7 +64,8 @@ ToolbarButton.displayName = "ToolbarButton";

const toolbarItems = TOOLBAR_ITEMS.document;

export const PageToolbar: React.FC<Props> = ({ editorRef }) => {
export const PageToolbar: React.FC<Props> = (props) => {
const { editorRef, isHidden } = props;
// states
const [activeStates, setActiveStates] = useState<Record<string, boolean>>({});

Expand Down Expand Up @@ -96,7 +98,14 @@ export const PageToolbar: React.FC<Props> = ({ editorRef }) => {
);

return (
<div className="flex flex-wrap items-center divide-x divide-custom-border-200">
<div
className={cn(
"flex flex-wrap items-center divide-x divide-custom-border-200 opacity-100 transition-opacity duration-200 ease-in-out",
{
"opacity-0 pointer-events-none": isHidden,
}
)}
>
<CustomMenu
customButton={
<span className="text-custom-text-300 text-sm border-[0.5px] border-custom-border-300 hover:bg-custom-background-80 h-7 w-24 rounded px-2 flex items-center justify-between gap-2 whitespace-nowrap text-left">
Expand Down