Przeglądaj źródła

fix layering for PageControls and PagePathNav

Yuki Takei 3 miesięcy temu
rodzic
commit
ddc1bf522a

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

@@ -0,0 +1,88 @@
+# 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` - 2つの Sticky に分離
+  - 1つ目: GroundGlassBar のみ(`position-fixed`, `z-1`)
+  - 2つ目: nav 要素(`z-3`)
+- `PagePathNavSticky.tsx` - z-index を動的に切り替え
+  - 通常時: `z-2`
+  - sticky時: `z-3`
+
+## 実装のポイント
+
+### GroundGlassBar を別の Sticky に分離した理由
+GroundGlassBar を `position-fixed` で常に固定表示にすることで、
+PageControls と切り離して独立した z-index 層として扱えるようにした。
+
+これにより、GroundGlassBar → PagePathNav → PageControls という
+理想的なレイヤー構造を実現できた。
+
+### 代替手段の検討
+現在の「2つの Sticky に分離する」実装は動作するが、
+よりクリーンな代替手段があれば検討の余地がある。
+
+## 未解決の問題(要調査)
+
+### CopyDropdown が z-2 で動作しない問題
+
+`PagePathNavSticky.tsx` の sticky 時の z-index について:
+
+```tsx
+// これだと CopyDropdown(マウスオーバーで表示されるドロップダウン)が出ない
+innerActiveClass="active z-2 mt-1"
+
+// これだと正常に動作する
+innerActiveClass="active z-3 mt-1"
+```
+
+**原因は不明。** 将来的に調査が必要。
+
+考えられる可能性:
+- CopyDropdown のドロップダウンメニュー自体の z-index との関係
+- 他の要素が z-2 で被さっている可能性
+- GrowiContextualSubNavigation の nav 要素(z-3)との干渉
+
+## 関連ファイル
+
+- `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 を含む)
+
+## 注意事項
+
+- z-index の値を変更する際は、上記のレイヤー構造を壊さないよう注意
+- Sticky コンポーネントの `innerActiveClass` で z-index を指定する際、
+  他のコンポーネントとの相互作用を確認すること

+ 1 - 5
apps/app/src/client/components/Navbar/GrowiContextualSubNavigation.module.scss

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

+ 66 - 60
apps/app/src/client/components/Navbar/GrowiContextualSubNavigation.tsx

@@ -59,6 +59,8 @@ import { Skeleton } from '../Skeleton';
 import styles from './GrowiContextualSubNavigation.module.scss';
 import styles from './GrowiContextualSubNavigation.module.scss';
 import PageEditorModeManagerStyles from './PageEditorModeManager.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(
 const PageEditorModeManager = dynamic(
   () => import('./PageEditorModeManager').then(mod => mod.PageEditorModeManager),
   () => import('./PageEditorModeManager').then(mod => mod.PageEditorModeManager),
@@ -387,72 +389,76 @@ const GrowiContextualSubNavigation = (props: GrowiContextualSubNavigationProps):
       <GroundGlassBar className="py-4 d-block d-md-none d-print-none border-bottom" />
       <GroundGlassBar className="py-4 d-block d-md-none d-print-none border-bottom" />
 
 
       <Sticky
       <Sticky
-        className="z-1"
+        className="z-1 position-fixed d-print-none"
         enabled={!isPrinting}
         enabled={!isPrinting}
-        onStateChange={status => setStickyActive(status.status === Sticky.STATUS_FIXED)}
         innerActiveClass="w-100 end-0"
         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"
-          >
+        <GroundGlassBar className={minHeightSubNavigation} />
+      </Sticky>
 
 
-            <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}
+      <Sticky
+        className="z-3"
+        enabled={!isPrinting}
+        onStateChange={status => setStickyActive(status.status === Sticky.STATUS_FIXED)}
+        innerActiveClass="w-100 end-0"
+      >
+        <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>
-                <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>
                 </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>
       </Sticky>
 
 
       {path != null && currentUser != null && !isReadOnlyUser && (
       {path != null && currentUser != null && !isReadOnlyUser && (

+ 0 - 8
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 {
 .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 {
   .sticky-inner-wrapper.active {
     h1 {
     h1 {
       font-size: 1.75rem !important;
       font-size: 1.75rem !important;

+ 1 - 1
apps/app/src/client/components/PagePathNavSticky/PagePathNavSticky.tsx

@@ -84,7 +84,7 @@ export const PagePathNavSticky = (props: PagePathNavLayoutProps): JSX.Element =>
     // Controlling pointer-events
     // Controlling pointer-events
     //  1. disable pointer-events with 'pe-none'
     //  1. disable pointer-events with 'pe-none'
     <div ref={pagePathNavRef}>
     <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 }) => {
         {({ status }) => {
           const isStatusFixed = status === Sticky.STATUS_FIXED;
           const isStatusFixed = status === Sticky.STATUS_FIXED;