Browse Source

fix(spec): impl v2 fix unread dots, dock scroll, and instant read state

Ryotaro Nagahara 1 month ago
parent
commit
33677ca040

+ 27 - 0
.kiro/specs/news-inappnotification/design.md

@@ -399,6 +399,12 @@ type FilterType = 'all' | 'news' | 'notifications';
 
 ---
 
+#### InAppNotificationElm.tsx(既存・修正あり)
+
+**実装後に判明した落とし穴**: 未読ドットに使われていた CSS クラス `grw-unopend-notification` はコードベースに定義が存在せず、ドットが不可視だった。`bg-primary rounded-circle` + インラインスタイル(`width/height: 8px, display: inline-block`)に置き換えて修正済み。このコンポーネントを今後変更する場合、同クラスを再導入しないこと。
+
+---
+
 #### InAppNotificationSubstance.tsx(変更)
 
 | Field | Detail |
@@ -420,6 +426,27 @@ type FilterType = 'all' | 'news' | 'notifications';
 - 既存 `InfiniteScroll` コンポーネントを使用(`client/components/InfiniteScroll.tsx`)
 - 既存 `// TODO: Infinite scroll implemented` コメントを解消
 
+**サイドバーモード別スクロール戦略(実装後に判明した設計上の決定)**:
+
+サイドバーには2種類のモードがあり、スクロール担当コンテナが異なる。
+
+| モード | UI | スクロール担当 | コンテンツエリアの制約 |
+|---|---|---|---|
+| collapsed(ホバーパネル ①) | ベルアイコンにホバー時の小パネル | `InAppNotificationContent` 内の `overflow-auto` div | `maxHeight: 60vh` で高さを制限 |
+| dock / drawer(全面サイドバー ②) | 展開した全面パネル | 外側の `SimpleBar`(`h-100`) | 制約なし。コンテンツが自然に伸長 |
+
+collapsed モードで `overflow-auto + maxHeight` を使い、dock/drawer モードでは外していない場合、**二重スクロールコンテナ**が発生する。具体的には:
+- `overflow-auto` div がサイドバーと同高の scroll context を作る
+- スクロールバーがコンテンツ高さとほぼ同じ縦幅で出現し、わずかな余白でしか動かせなくなる(振動挙動)
+
+対策として `InAppNotificationContent` 内で `useSidebarMode()` を呼び、`isCollapsedMode()` が true のときのみ `overflow-auto` クラスと `maxHeight: 60vh` を付与する。dock/drawer モードでは div に何も付与せず、SimpleBar にスクロールを委ねる。
+
+**通知ドット即時消去のローカル state 戦略(実装後に判明した設計上の決定)**:
+
+`InAppNotificationElm` はクリック時に `apiv3Post('/in-app-notification/open')` でサーバーへ書き込みを行うが、UI への反映は SWR の再フェッチに依存する。`InAppNotificationContent` 内で `useSWRInfinite` の `mutate(updater, { revalidate: false })` を使って楽観的更新を試みたが、ナビゲーション(`<a href>`)でコンポーネントがアンマウントされると `useSWRInfinite` のページ単位キャッシュが古い状態に戻り、再マウント時にドットが復活する問題があった。
+
+対策として `InAppNotificationContent` に `useState<Set<string>>` を持ち、ユーザーがクリックした通知 ID をローカルに記録する。各 `InAppNotificationElm` のレンダリング時にこの set を参照し、ID が含まれる場合は `notification.status` を `STATUS_OPENED` にオーバーライドして渡す。これにより SWR キャッシュの状態によらず確実に即時反映される。
+
 ---
 
 #### NewsItem Component

+ 1 - 1
.kiro/specs/news-inappnotification/requirements.md

@@ -71,7 +71,7 @@ GROWI の InAppNotification にニュース配信・表示機能を追加する
 1. The InAppNotificationパネル shall 通知とニュースを公開日時/作成日時の降順で混合した1つのリストとして表示する
 2. The InAppNotificationパネル shall 上部にフィルタボタン(「すべて」「通知」「お知らせ」)を配置し、デフォルトは「すべて」とする。「お知らせ」選択時はニュースのみ、「通知」選択時はニュース以外のすべての通知を表示する
 3. The InAppNotificationパネル shall 既存の「未読のみ」トグルスイッチを維持し、種別フィルタと組み合わせた2重フィルタリングを提供する。種別フィルタ(すべて/通知/お知らせ)で表示対象を絞り込んだ上で、トグルON時は未読アイテムのみをさらに絞り込む
-4. The InAppNotificationパネル shall リスト領域に最大高さを設定し、超過分はスクロールで表示する。スクロールが末端に達した場合は次のページを自動で読み込む無限スクロールとする
+4. The InAppNotificationパネル shall リスト領域のスクロールを提供し、末端に達した場合は次のページを自動で読み込む無限スクロールとする。スクロールの実現方法はサイドバーモードに依存する:collapsed モード(ホバーパネル)では最大高さ(`60vh`)を設定した内部スクロールコンテナを使用し、dock/drawer モード(全面サイドバー)では外側の SimpleBar コンテナにスクロールを委ねることで二重スクロールコンテナを回避する
 5. The InAppNotificationパネル shall ニュースアイテムの `emoji` フィールドをタイトル前に表示する。`emoji` 未設定の場合は 📢 をフォールバックとして使用する
 6. When ユーザーがニュースアイテムをクリックした場合, the InAppNotification UI shall ニュースの詳細 URL を新しいタブで開く
 7. When ユーザーがニュースアイテムをクリックした場合, the InAppNotification UI shall 該当ニュースを既読としてマークし、未読インジケータを更新する

+ 16 - 0
.kiro/specs/news-inappnotification/tasks.md

@@ -148,3 +148,19 @@
   - フィルタタブ切り替えで表示対象が変わることを確認する(5.2 の AC カバレッジ)
   - 「未読のみ」トグルとの組み合わせで2重フィルタリングが機能することを確認する(5.3 の AC カバレッジ)
   - _Requirements: 5.2, 5.3_
+
+- [x] 12. 既存コードの不具合修正(実装後検証で発覚)
+- [x] 12.1 既存通知の未読ドットを修正する
+  - `InAppNotificationElm.tsx` の `grw-unopend-notification` クラスに対応する CSS 定義がコードベースに存在しないため、未読ドットが表示されない
+  - NewsItem と同様に `width/height/display: inline-block` のインラインスタイルを追加する
+  - _Requirements: 6.1_
+
+- [x] 12.2 全面サイドバー(② dock/drawer モード)での通知表示エリアを拡張する
+  - `InAppNotificationSubstance.tsx` の各フィルタ表示エリアに `style={{ maxHeight: '60vh' }}` が固定されており、② dock/drawer モードでもホバーパネル(①)サイズに制限される
+  - `useSidebarMode()` で collapsed モードを判定し、collapsed 時のみ `maxHeight: '60vh'` を適用する。dock/drawer モードでは制約を外し、外側の SimpleBar コンテナによるスクロールに委ねる
+  - _Requirements: 5.1_
+
+- [x] 12.3 アプリ内通知の未読ドットをクリック時に即時消去する
+  - `InAppNotificationSubstance.tsx` の `handleNotificationRead` で `useSWRInfinite` の `mutate(updater, { revalidate: false })` を使って既読状態をキャッシュに書き込もうとしていたが、ナビゲーション(`<a href>`)によってコンポーネントがアンマウントされた後に `useSWRInfinite` のページ単位キャッシュが古い状態に戻るため、ドットが再表示される
+  - `useState<Set<string>>` でローカルに開封済み通知 ID を管理し、各 `InAppNotificationElm` のレンダリング時に `status` をその場でオーバーライドすることで、SWR キャッシュに依存せず即時反映を実現する
+  - _Requirements: 6.1, 6.2_

+ 17 - 0
apps/app/run-news-cron.mts

@@ -0,0 +1,17 @@
+// biome-ignore-all lint/suspicious/noConsole: dev script
+import mongoose from 'mongoose';
+
+import { NewsCronService } from './src/features/news/server/services/news-cron-service.js';
+
+const MONGO_URI = process.env.MONGO_URI ?? 'mongodb://mongo:27017/growi';
+process.env.NEWS_FEED_URL ??= 'http://localhost:8099/feed.json';
+process.env.GROWI_SKIP_NEWS_SLEEP = 'true';
+
+console.log(`Connecting to ${MONGO_URI}...`);
+await mongoose.connect(MONGO_URI);
+console.log('Connected. Running cron job...');
+
+await new NewsCronService().executeJob();
+
+console.log('Done.');
+await mongoose.disconnect();

+ 9 - 3
apps/app/src/client/components/InAppNotification/InAppNotificationElm.tsx

@@ -77,10 +77,16 @@ const InAppNotificationElm: FC<Props> = (props: Props) => {
           <span
             className={`${
               notification.status === InAppNotificationStatuses.STATUS_UNOPENED
-                ? 'grw-unopend-notification'
-                : 'ms-2'
+                ? 'bg-primary'
+                : ''
             } rounded-circle me-3`}
-          ></span>
+            style={{
+              width: 8,
+              height: 8,
+              minWidth: 8,
+              display: 'inline-block',
+            }}
+          />
 
           {renderActionUserPictures()}
 

+ 60 - 13
apps/app/src/client/components/Sidebar/InAppNotification/InAppNotificationSubstance.tsx

@@ -1,14 +1,18 @@
-import { type JSX, useId, useMemo } from 'react';
+import { type JSX, useId, useMemo, useState } from 'react';
 import type { HasObjectId } from '@growi/core';
 import { useTranslation } from 'next-i18next';
 
 import InAppNotificationElm from '~/client/components/InAppNotification/InAppNotificationElm';
 import InfiniteScroll from '~/client/components/InfiniteScroll';
 import { NewsItem } from '~/features/news/client/components/NewsItem';
-import { useSWRINFxNews } from '~/features/news/client/hooks/use-news';
+import {
+  useSWRINFxNews,
+  useSWRxNewsUnreadCount,
+} from '~/features/news/client/hooks/use-news';
 import type { INewsItemWithReadStatus } from '~/features/news/interfaces/news-item';
 import type { IInAppNotification } from '~/interfaces/in-app-notification';
 import { InAppNotificationStatuses } from '~/interfaces/in-app-notification';
+import { useSidebarMode } from '~/states/ui/sidebar';
 import { useSWRINFxInAppNotifications } from '~/stores/in-app-notification';
 
 import type { FilterType } from './InAppNotification';
@@ -98,6 +102,19 @@ export const InAppNotificationContent = (
 ): JSX.Element => {
   const { isUnopendNotificationsVisible, activeFilter } = props;
   const { t } = useTranslation('commons');
+  const { isCollapsedMode } = useSidebarMode();
+
+  // Track locally-opened notifications to give instant dot removal without
+  // relying on SWR cache persistence across navigation/unmount cycles.
+  const [locallyOpenedNotifIds, setLocallyOpenedNotifIds] = useState<
+    Set<string>
+  >(new Set());
+
+  // In collapsed mode (hover panel): constrain height + own scrollbar
+  // In dock/drawer mode: no constraints — outer SimpleBar handles all scrolling
+  const collapsed = isCollapsedMode();
+  const scrollAreaClassName = collapsed ? 'overflow-auto' : undefined;
+  const scrollAreaStyle = collapsed ? { maxHeight: '60vh' } : undefined;
 
   const notificationStatus = isUnopendNotificationsVisible
     ? InAppNotificationStatuses.STATUS_UNOPENED
@@ -109,6 +126,7 @@ export const InAppNotificationContent = (
     { onlyUnread: isUnopendNotificationsVisible },
     { keepPreviousData: true },
   );
+  const { mutate: mutateNewsUnreadCount } = useSWRxNewsUnreadCount();
 
   const notificationResponse = useSWRINFxInAppNotifications(
     NEWS_PER_PAGE,
@@ -210,6 +228,15 @@ export const InAppNotificationContent = (
 
   const handleReadMutate = () => {
     newsResponse.mutate();
+    mutateNewsUnreadCount();
+  };
+
+  // Use local state to immediately remove the unread dot on click.
+  // Relying solely on SWR mutate is unreliable because useSWRInfinite per-page
+  // caches can be stale after navigation/unmount, so the dot reappears on
+  // remount even with revalidate:false.
+  const handleNotificationRead = (notificationId: string) => {
+    setLocallyOpenedNotifIds((prev) => new Set(prev).add(notificationId));
   };
 
   if (activeFilter === 'news') {
@@ -218,7 +245,7 @@ export const InAppNotificationContent = (
     }
 
     return (
-      <div className="overflow-auto" style={{ maxHeight: '60vh' }}>
+      <div className={scrollAreaClassName} style={scrollAreaStyle}>
         <InfiniteScroll
           swrInifiniteResponse={newsResponse}
           isReachingEnd={newsExhausted}
@@ -246,18 +273,29 @@ export const InAppNotificationContent = (
     }
 
     return (
-      <div className="overflow-auto" style={{ maxHeight: '60vh' }}>
+      <div className={scrollAreaClassName} style={scrollAreaStyle}>
         <InfiniteScroll
           swrInifiniteResponse={notificationResponse}
           isReachingEnd={notifExhausted}
         >
           <div className="list-group">
-            {allNotificationItems.map((notification) => (
-              <InAppNotificationElm
-                key={notification._id.toString()}
-                notification={notification}
-              />
-            ))}
+            {allNotificationItems.map((notification) => {
+              const id = notification._id.toString();
+              return (
+                <InAppNotificationElm
+                  key={id}
+                  notification={
+                    locallyOpenedNotifIds.has(id)
+                      ? {
+                          ...notification,
+                          status: InAppNotificationStatuses.STATUS_OPENED,
+                        }
+                      : notification
+                  }
+                  onUnopenedNotificationOpend={() => handleNotificationRead(id)}
+                />
+              );
+            })}
           </div>
         </InfiniteScroll>
       </div>
@@ -274,7 +312,7 @@ export const InAppNotificationContent = (
   }
 
   return (
-    <div className="overflow-auto" style={{ maxHeight: '60vh' }}>
+    <div className={scrollAreaClassName} style={scrollAreaStyle}>
       <InfiniteScroll
         swrInifiniteResponse={
           allModeSWRResponse as unknown as Parameters<
@@ -294,10 +332,19 @@ export const InAppNotificationContent = (
                 />
               );
             }
+            const id = entry.item._id.toString();
             return (
               <InAppNotificationElm
-                key={`notif-${entry.item._id.toString()}`}
-                notification={entry.item}
+                key={`notif-${id}`}
+                notification={
+                  locallyOpenedNotifIds.has(id)
+                    ? {
+                        ...entry.item,
+                        status: InAppNotificationStatuses.STATUS_OPENED,
+                      }
+                    : entry.item
+                }
+                onUnopenedNotificationOpend={() => handleNotificationRead(id)}
               />
             );
           })}

+ 4 - 1
apps/app/src/features/news/server/services/news-cron-service.ts

@@ -93,7 +93,10 @@ export class NewsCronService extends CronService {
     }
 
     // Random sleep to distribute requests across multiple GROWI instances
-    await randomSleep(MAX_RANDOM_SLEEP_MS);
+    // Skip in development/testing via GROWI_SKIP_NEWS_SLEEP=true
+    if (process.env.GROWI_SKIP_NEWS_SLEEP !== 'true') {
+      await randomSleep(MAX_RANDOM_SLEEP_MS);
+    }
 
     let feedJson: FeedJson;
     try {