Merge pull request #10665 from growilabs/imprv/page-path-nav-title-spacing

imprv: PagePathNavTitle spacing and z-index layering

.serena/memories/apps-app-page-path-nav-and-sub-navigation-layering.md

@@ -0,0 +1,105 @@
+# PagePathNav と SubNavigation の z-index レイヤリング
+
+## 概要
+
+PagePathNav（ページパス表示）と GrowiContextualSubNavigation（PageControls等を含むサブナビゲーション）の
+Sticky 状態における z-index の重なり順を修正した際の知見。
+
+## 修正したバグ
+
+### 症状
+スクロールしていって PagePathNav がウィンドウ上端に近づいたときに、PageControls のボタンが
+PagePathNav の要素の裏側に回ってしまい、クリックできなくなる。
+
+### 原因
+z-index 的に以下のように重なっていたため：
+
+**[Before]** 下層から順に：
+1. PageView の children - z-0
+2. ( GroundGlassBar = PageControls ) ← 同じ層 z-1
+3. PagePathNav
+
+PageControls が PagePathNav より下層にいたため、sticky 境界付近でクリック不能になっていた。
+
+## 修正後の構成
+
+**[After]** 下層から順に：
+1. PageView の children - z-0
+2. GroundGlassBar（磨りガラス背景）- z-1
+3. PagePathNav - z-2（通常時）/ z-3（sticky時）
+4. PageControls（nav要素）- z-3
+
+### ファイル構成
+
+- `GrowiContextualSubNavigation.tsx` - GroundGlassBar を分離してレンダリング
+  - 1つ目: GroundGlassBar のみ（`position-fixed`, `z-1`）
+  - 2つ目: nav 要素（`z-3`）
+- `PagePathNavSticky.tsx` - z-index を動的に切り替え
+  - 通常時: `z-2`
+  - sticky時: `z-3`
+
+## 実装のポイント
+
+### GroundGlassBar を分離した理由
+GroundGlassBar を `position-fixed` で常に固定表示にすることで、
+PageControls と切り離して独立した z-index 層として扱えるようにした。
+
+これにより、GroundGlassBar → PagePathNav → PageControls という
+理想的なレイヤー構造を実現できた。
+
+## CopyDropdown が z-2 で動作しない理由（解決済み）
+
+### 問題
+
+`PagePathNavSticky.tsx` の sticky 時の z-index について：
+
+```tsx
+// これだと CopyDropdown（マウスオーバーで表示されるドロップダウン）が出ない
+innerActiveClass="active z-2 mt-1"
+
+// これだと正常に動作する
+innerActiveClass="active z-3 mt-1"
+```
+
+### 原因
+
+1. `GrowiContextualSubNavigation` の sticky-inner-wrapper は `z-3` かつ横幅いっぱい(Flex アイテム)
+2. この要素が PagePathNavSticky（`z-2`）の上に重なる
+3. CopyDropdown は `.grw-page-path-nav-layout:hover` で `visibility: visible` になる仕組み
+   （参照: `PagePathNavLayout.module.scss`）
+4. **z-3 の要素が上に被さっているため、hover イベントが PagePathNavSticky に届かない**
+5. 結果、CopyDropdown のアイコンが表示されない
+
+### なぜ z-3 で動作するか
+
+- 同じ z-index: 3 になるため、DOM 順序で前後が決まる
+- PagePathNavSticky は GrowiContextualSubNavigation より後にレンダリングされるため前面に来る
+- hover イベントが正常に届き、CopyDropdown が表示される
+
+### 結論
+
+PagePathNavSticky の sticky 時の z-index は `z-3` である必要がある。
+これは GrowiContextualSubNavigation と同じ層に置くことで、DOM 順序による前後関係を利用するため。
+
+## 関連ファイル
+
+- `apps/app/src/client/components/PageView/PageView.tsx`
+- `apps/app/src/client/components/Navbar/GrowiContextualSubNavigation.tsx`
+- `apps/app/src/client/components/Navbar/GrowiContextualSubNavigation.module.scss`
+- `apps/app/src/client/components/PagePathNavSticky/PagePathNavSticky.tsx`
+- `apps/app/src/client/components/PagePathNavSticky/PagePathNavSticky.module.scss`
+- `apps/app/src/components/Common/PagePathNav/PagePathNavLayout.tsx`（CopyDropdown を含む）
+
+## ライブラリの注意事項
+
+### react-stickynode の deprecation
+`react-stickynode` は **2025-12-31 で deprecated** となる予定。
+https://github.com/yahoo/react-stickynode
+
+将来的には CSS `position: sticky` + `IntersectionObserver` への移行を検討する必要がある。
+
+## 注意事項
+
+- z-index の値を変更する際は、上記のレイヤー構造を壊さないよう注意
+- Sticky コンポーネントの `innerActiveClass` で z-index を指定する際、
+  他のコンポーネントとの相互作用を確認すること

apps/app/src/client/components/Navbar/GrowiContextualSubNavigation.module.scss

@@ -1,12 +1,8 @@
 @use '~/styles/mixins';
 @use '@growi/core-styles/scss/bootstrap/init' as bs;
 
-.grw-contextual-sub-navigation {
+.grw-min-height-sub-navigation {
   min-height: 46px;
-
-  @include bs.media-breakpoint-up(lg) {
-    min-height: 46px;
-  }
 }
 
 @include mixins.at-editing() {

apps/app/src/client/components/Navbar/GrowiContextualSubNavigation.tsx

@@ -73,6 +73,9 @@ import { Skeleton } from '../Skeleton';
 import styles from './GrowiContextualSubNavigation.module.scss';
 import PageEditorModeManagerStyles from './PageEditorModeManager.module.scss';
 
+const moduleClass = styles['grw-contextual-sub-navigation'];
+const minHeightSubNavigation = styles['grw-min-height-sub-navigation'];
+
 const PageEditorModeManager = dynamic(
   () =>
     import('./PageEditorModeManager').then((mod) => mod.PageEditorModeManager),
@@ -456,90 +459,94 @@ const GrowiContextualSubNavigation = (
 
   return (
     <>
+      {/* for App Title for mobile */}
       <GroundGlassBar className="py-4 d-block d-md-none d-print-none border-bottom" />
 
+      {/* for Sub Navigation */}
+      <GroundGlassBar
+        className={`position-fixed z-1 d-edit-none d-print-none w-100 end-0 ${minHeightSubNavigation}`}
+      />
+
       <Sticky
-        className="z-1"
+        className="z-3"
         enabled={!isPrinting}
         onStateChange={(status) =>
           setStickyActive(status.status === Sticky.STATUS_FIXED)
         }
         innerActiveClass="w-100 end-0"
       >
-        <GroundGlassBar>
-          <nav
-            className={`${styles['grw-contextual-sub-navigation']}
-              d-flex align-items-center justify-content-end pe-2 pe-sm-3 pe-md-4 py-1 gap-2 gap-md-4 d-print-none
-            `}
-            data-testid="grw-contextual-sub-nav"
-            id="grw-contextual-sub-nav"
-          >
-            <PageControls
-              pageId={pageId}
-              revisionId={revisionId}
-              shareLinkId={shareLinkId}
-              path={path ?? currentPathname} // If the page is empty, "path" is undefined
-              expandContentWidth={shouldExpandContent}
-              disableSeenUserInfoPopover={isSharedUser}
-              hideSubControls={hideSubControls}
-              showPageControlDropdown={isAbleToShowPageManagement}
-              additionalMenuItemRenderer={additionalMenuItemsRenderer}
-              onClickDuplicateMenuItem={duplicateItemClickedHandler}
-              onClickRenameMenuItem={renameItemClickedHandler}
-              onClickDeleteMenuItem={deleteItemClickedHandler}
-              onClickSwitchContentWidth={switchContentWidthHandler}
+        <nav
+          className={`${moduleClass} ${minHeightSubNavigation}
+            d-flex align-items-center justify-content-end pe-2 pe-sm-3 pe-md-4 py-1 gap-2 gap-md-4 d-print-none
+          `}
+          data-testid="grw-contextual-sub-nav"
+          id="grw-contextual-sub-nav"
+        >
+          <PageControls
+            pageId={pageId}
+            revisionId={revisionId}
+            shareLinkId={shareLinkId}
+            path={path ?? currentPathname} // If the page is empty, "path" is undefined
+            expandContentWidth={shouldExpandContent}
+            disableSeenUserInfoPopover={isSharedUser}
+            hideSubControls={hideSubControls}
+            showPageControlDropdown={isAbleToShowPageManagement}
+            additionalMenuItemRenderer={additionalMenuItemsRenderer}
+            onClickDuplicateMenuItem={duplicateItemClickedHandler}
+            onClickRenameMenuItem={renameItemClickedHandler}
+            onClickDeleteMenuItem={deleteItemClickedHandler}
+            onClickSwitchContentWidth={switchContentWidthHandler}
+          />
+
+          {isAbleToChangeEditorMode && (
+            <PageEditorModeManager
+              editorMode={editorMode}
+              isBtnDisabled={!!isGuestUser || !!isReadOnlyUser}
+              path={path}
             />
+          )}
 
-            {isAbleToChangeEditorMode && (
-              <PageEditorModeManager
-                editorMode={editorMode}
-                isBtnDisabled={!!isGuestUser || !!isReadOnlyUser}
-                path={path}
-              />
-            )}
-
-            {isGuestUser && (
-              <div className="mt-2">
-                <span>
-                  <span className="d-inline-block" id="sign-up-link">
-                    <Link
-                      href={
-                        !isLocalAccountRegistrationEnabled
-                          ? '#'
-                          : '/login#register'
-                      }
-                      className={`btn me-2 ${!isLocalAccountRegistrationEnabled ? 'opacity-25' : ''}`}
-                      style={{
-                        pointerEvents: !isLocalAccountRegistrationEnabled
-                          ? 'none'
-                          : undefined,
-                      }}
-                      prefetch={false}
-                    >
-                      <span className="material-symbols-outlined me-1">
-                        person_add
-                      </span>
-                      {t('Sign up')}
-                    </Link>
-                  </span>
-                  {!isLocalAccountRegistrationEnabled && (
-                    <UncontrolledTooltip target="sign-up-link" fade={false}>
-                      {t('tooltip.login_required')}
-                    </UncontrolledTooltip>
-                  )}
+          {isGuestUser && (
+            <div>
+              <span>
+                <span className="d-inline-block" id="sign-up-link">
+                  <Link
+                    href={
+                      !isLocalAccountRegistrationEnabled
+                        ? '#'
+                        : '/login#register'
+                    }
+                    className={`btn me-2 ${!isLocalAccountRegistrationEnabled ? 'opacity-25' : ''}`}
+                    style={{
+                      pointerEvents: !isLocalAccountRegistrationEnabled
+                        ? 'none'
+                        : undefined,
+                    }}
+                    prefetch={false}
+                  >
+                    <span className="material-symbols-outlined me-1">
+                      person_add
+                    </span>
+                    {t('Sign up')}
+                  </Link>
                 </span>
-                <Link
-                  href="/login#login"
-                  className="btn btn-primary"
-                  prefetch={false}
-                >
-                  <span className="material-symbols-outlined me-1">login</span>
-                  {t('Sign in')}
-                </Link>
-              </div>
-            )}
-          </nav>
-        </GroundGlassBar>
+                {!isLocalAccountRegistrationEnabled && (
+                  <UncontrolledTooltip target="sign-up-link" fade={false}>
+                    {t('tooltip.login_required')}
+                  </UncontrolledTooltip>
+                )}
+              </span>
+              <Link
+                href="/login#login"
+                className="btn btn-primary"
+                prefetch={false}
+              >
+                <span className="material-symbols-outlined me-1">login</span>
+                {t('Sign in')}
+              </Link>
+            </div>
+          )}
+        </nav>
       </Sticky>
 
       {path != null && currentUser != null && !isReadOnlyUser && (

apps/app/src/client/components/PagePathNavSticky/PagePathNavSticky.module.scss

@@ -1,12 +1,4 @@
-@use '@growi/core-styles/scss/bootstrap/init' as bs;
-
 .grw-page-path-nav-sticky :global {
-  .sticky-inner-wrapper {
-    z-index: bs.$zindex-sticky;
-  }
-
-  // TODO:Responsive font size
-  // set smaller font-size when sticky
   .sticky-inner-wrapper.active {
     h1 {
       font-size: 1.75rem !important;

apps/app/src/client/components/PagePathNavSticky/PagePathNavSticky.tsx

@@ -25,7 +25,7 @@ const { isTrashPage } = pagePathUtils;
 
 
 export const PagePathNavSticky = (props: PagePathNavLayoutProps): JSX.Element => {
-  const { pagePath } = props;
+  const { pagePath, latterLinkClassName, ...rest } = props;
 
   const isPrinting = usePrintMode();
 
@@ -84,33 +84,42 @@ export const PagePathNavSticky = (props: PagePathNavLayoutProps): JSX.Element =>
     // Controlling pointer-events
     //  1. disable pointer-events with 'pe-none'
     <div ref={pagePathNavRef}>
-      <Sticky className={moduleClass} enabled={!isPrinting} innerClass="pe-none" innerActiveClass="active mt-1">
+      <Sticky className={moduleClass} enabled={!isPrinting} innerClass="z-2 pe-none" innerActiveClass="active z-3 mt-1">
         {({ status }) => {
-          const isParentsCollapsed = status === Sticky.STATUS_FIXED;
-
-          // Controlling pointer-events
-          //  2. enable pointer-events with 'pe-auto' only against the children
-          //      which width is minimized by 'd-inline-block'
-          //
-          if (isParentsCollapsed) {
-            return (
-              <div className="d-inline-block pe-auto">
-                <PagePathNavLayout
-                  {...props}
-                  latterLink={latterLink}
-                  latterLinkClassName="fs-3 text-truncate"
-                  maxWidth={navMaxWidth}
-                />
-              </div>
-            );
-          }
+          const isStatusFixed = status === Sticky.STATUS_FIXED;
 
           return (
-            // Use 'd-block' to make the children take the full width
-            // This is to improve UX when opening/closing CopyDropdown
-            <div className="d-block pe-auto">
-              <PagePathNav {...props} inline />
-            </div>
+            <>
+              {/*
+                * Controlling pointer-events
+                * 2. enable pointer-events with 'pe-auto' only against the children
+                *      which width is minimized by 'd-inline-block'
+                */}
+              { isStatusFixed && (
+                <div className="d-inline-block pe-auto position-absolute">
+                  <PagePathNavLayout
+                    pagePath={pagePath}
+                    latterLink={latterLink}
+                    latterLinkClassName={`${latterLinkClassName} text-truncate`}
+                    maxWidth={navMaxWidth}
+                    {...rest}
+                  />
+                </div>
+              )}
+
+              {/*
+                * Use 'd-block' to make the children take the full width
+                * This is to improve UX when opening/closing CopyDropdown
+                */}
+              <div className={`d-block pe-auto ${isStatusFixed ? 'invisible' : ''}`}>
+                <PagePathNav
+                  pagePath={pagePath}
+                  latterLinkClassName={latterLinkClassName}
+                  inline
+                  {...rest}
+                />
+              </div>
+            </>
           );
         }}
       </Sticky>

apps/app/src/client/components/TrashPageList.tsx

@@ -123,7 +123,7 @@ export const TrashPageList = (): JSX.Element => {
   }, [t]);
 
   return (
-    <div data-testid="trash-page-list" className="mt-5 d-edit-none">
+    <div data-testid="trash-page-list" className="d-edit-none">
       <CustomNavAndContents
         navTabMapping={navTabMapping}
         navRightElement={emptyTrashButton}

apps/app/src/components/Common/PagePathNavTitle/PagePathNavTitle.module.scss

@@ -1,5 +0,0 @@
-@use '@growi/core-styles/scss/bootstrap/init' as bs;
-
-.grw-page-path-nav-title :global {
-  min-height: 75px;
-}

apps/app/src/components/Common/PagePathNavTitle/PagePathNavTitle.tsx

@@ -6,10 +6,6 @@ import { useIsomorphicLayoutEffect } from 'usehooks-ts';
 import type { PagePathNavLayoutProps } from '../PagePathNav';
 import { PagePathNav } from '../PagePathNav';
 
-import styles from './PagePathNavTitle.module.scss';
-
-const moduleClass = styles['grw-page-path-nav-title'] ?? '';
-
 const PagePathNavSticky = withLoadingProps<PagePathNavLayoutProps>(
   (useLoadingProps) =>
     dynamic(
@@ -42,15 +38,9 @@ export const PagePathNavTitle = (
     setClient(true);
   }, []);
 
-  const className = `${moduleClass} mb-4`;
-
   return isClient ? (
-    <PagePathNavSticky
-      {...props}
-      className={className}
-      latterLinkClassName="fs-2"
-    />
+    <PagePathNavSticky {...props} latterLinkClassName="fs-2" />
   ) : (
-    <PagePathNav {...props} className={className} latterLinkClassName="fs-2" />
+    <PagePathNav {...props} latterLinkClassName="fs-2" />
   );
 };

apps/app/src/components/PageView/PageContentFooter.tsx

@@ -27,7 +27,7 @@ export const PageContentFooter = (
   }
 
   return (
-    <div className={`${styles['page-content-footer']} my-4 pt-4 d-edit-none`}>
+    <div className={`${styles['page-content-footer']} py-4 d-edit-none`}>
       <div className="page-meta">
         <AuthorInfo
           user={creator}

apps/app/src/components/PageView/PageView.tsx

@@ -1,4 +1,12 @@
-import { type JSX, memo, useCallback, useEffect, useMemo, useRef } from 'react';
+import {
+  type JSX,
+  memo,
+  useCallback,
+  useEffect,
+  useId,
+  useMemo,
+  useRef,
+} from 'react';
 import dynamic from 'next/dynamic';
 import { isDeepEquals } from '@growi/core/dist/utils/is-deep-equals';
 import { isUsersHomepage } from '@growi/core/dist/utils/page-path-utils';
@@ -103,6 +111,8 @@ const PageViewComponent = (props: Props): JSX.Element => {
   const isNotCreatable = useIsNotCreatable();
   const isNotFoundMeta = usePageNotFound();
 
+  const contentContainerId = useId();
+
   const page = useCurrentPageData();
   const { data: viewOptions } = useViewOptions();
 
@@ -129,9 +139,7 @@ const PageViewComponent = (props: Props): JSX.Element => {
       return;
     }
 
-    const contentContainer = document.getElementById(
-      'page-view-content-container',
-    );
+    const contentContainer = document.getElementById(contentContainerId);
     if (contentContainer == null) return;
 
     const targetId = decodeURIComponent(hash.slice(1));
@@ -152,7 +160,7 @@ const PageViewComponent = (props: Props): JSX.Element => {
     observer.observe(contentContainer, { childList: true, subtree: true });
 
     return () => observer.disconnect();
-  }, [currentPageId]);
+  }, [currentPageId, contentContainerId]);
 
   // *******************************  end  *******************************
 
@@ -252,7 +260,7 @@ const PageViewComponent = (props: Props): JSX.Element => {
           {isUsersHomepagePath && page?.creator != null && (
             <UserInfo author={page.creator} />
           )}
-          <div id="page-view-content-container" className="flex-expand-vert">
+          <div id={contentContainerId} className="flex-expand-vert">
             <Contents />
           </div>
         </>

apps/app/src/components/PageView/PageViewLayout.tsx

@@ -36,7 +36,7 @@ export const PageViewLayout = (props: Props): JSX.Element => {
       <div
         className={`main ${className} ${pageViewLayoutClass} ${fluidLayoutClass} flex-expand-vert ps-sidebar`}
       >
-        <div className="container-lg wide-gutter-x-lg grw-container-convertible flex-expand-vert">
+        <div className="container-lg wide-gutter-x-lg grw-container-convertible flex-expand-vert gap-4">
           {headerContents != null && headerContents}
           {!isPrinting && sideContents != null ? (
             <div className="flex-expand-horiz gap-3 z-0">

apps/app/src/pages/trash/index.page.tsx

@@ -67,8 +67,10 @@ const TrashPage: NextPageWithLayout<Props> = (props: Props) => {
 
         <div className="main ps-sidebar">
           <div className="container-lg wide-gutter-x-lg">
-            <PagePathNavTitle pagePath="/trash" />
-            <TrashPageList />
+            <div className="d-flex flex-column gap-4">
+              <PagePathNavTitle pagePath="/trash" />
+              <TrashPageList />
+            </div>
           </div>
         </div>
       </div>