小田急テーマの停車中バー灰色表示とヘッダーデザインを改善#5741
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughHeaderOdakyuでタブレット判定に基づくdivider高さとグラデーション定義を更新。LineBoardEastはindex===0での分割を許可し、splitWidthをindexとisTabletで動的計算。NumberingIcon系にshouldGrayscaleプロップを追加/伝播して描画色を切替可能に。 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/HeaderOdakyu.tsx (1)
1-5:⚠️ Potential issue | 🟡 Minorインポートの順序を修正してください。
Biome CI がインポートの順序が正しくないことを報告しています。
biome check --applyを実行してインポートを整理してください。🔧 修正案
import React from 'react'; +import isTablet from '~/utils/isTablet'; import type { CommonHeaderProps } from './Header.types'; import type { HeaderEastThemeConfig } from './HeaderEast'; import { HeaderEast, tokyoMetroConfig } from './HeaderEast'; -import isTablet from '~/utils/isTablet';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/HeaderOdakyu.tsx` around lines 1 - 5, Imports in HeaderOdakyu.tsx are out of order; run the formatter to fix them: execute `biome check --apply` to automatically reorder imports, or manually reorder so grouped imports are: external packages (React), type-only imports (CommonHeaderProps, HeaderEastThemeConfig), component exports (HeaderEast, tokyoMetroConfig) and local utilities (isTablet) — referencing the symbols React, CommonHeaderProps, HeaderEastThemeConfig, HeaderEast, tokyoMetroConfig, and isTablet to locate and verify the changes.
🧹 Nitpick comments (1)
src/components/LineBoardEast.tsx (1)
235-254:dotCenterOffsetとsplitWidthの計算が重複しています。Lines 236-238 と Lines 250-252 で同じ計算が2回行われています。関数スコープ内で一度だけ計算し、再利用することを推奨します。
♻️ 重複を解消する修正案
if (splitHere) { // index 0ではバー左端からドット中心までの距離を灰色幅に使う const dotCenterOffset = isTablet ? 24 : 16; const splitWidth = index === 0 ? Math.abs(barLeft) + dotCenterOffset : barWidth / 2.5; gradients.push( createBarGradient( 'bar-main-half', getMainBarColors(line), barLeft, splitWidth ) ); - } - - if (secondaryVisible) { - const dotCenterOffset = isTablet ? 24 : 16; - const splitWidth = - index === 0 ? Math.abs(barLeft) + dotCenterOffset : barWidth / 2.5; - const left = splitHere ? barLeft + splitWidth : barLeft; - const width = splitHere ? barWidth - splitWidth : barWidth; + if (secondaryVisible) { + gradients.push( + createBarGradient( + 'bar-color', + getLineBarColors(line, lineColors, index), + barLeft + splitWidth, + barWidth - splitWidth + ) + ); + } + } else if (secondaryVisible) { gradients.push( createBarGradient( 'bar-color', getLineBarColors(line, lineColors, index), - left, - width + barLeft, + barWidth ) ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/LineBoardEast.tsx` around lines 235 - 254, dotCenterOffset and splitWidth are computed twice; compute them once in the enclosing function scope and reuse for both primary and secondary bar logic: move the dotCenterOffset (using isTablet) and splitWidth (using index, barLeft, barWidth) declarations out of the first gradient/secondaryVisible blocks so both the createBarGradient call and the secondaryVisible branch use the same variables; ensure splitHere and left/width calculations remain based on that single splitWidth and remove the duplicated declarations inside the secondaryVisible block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/components/HeaderOdakyu.tsx`:
- Around line 1-5: Imports in HeaderOdakyu.tsx are out of order; run the
formatter to fix them: execute `biome check --apply` to automatically reorder
imports, or manually reorder so grouped imports are: external packages (React),
type-only imports (CommonHeaderProps, HeaderEastThemeConfig), component exports
(HeaderEast, tokyoMetroConfig) and local utilities (isTablet) — referencing the
symbols React, CommonHeaderProps, HeaderEastThemeConfig, HeaderEast,
tokyoMetroConfig, and isTablet to locate and verify the changes.
---
Nitpick comments:
In `@src/components/LineBoardEast.tsx`:
- Around line 235-254: dotCenterOffset and splitWidth are computed twice;
compute them once in the enclosing function scope and reuse for both primary and
secondary bar logic: move the dotCenterOffset (using isTablet) and splitWidth
(using index, barLeft, barWidth) declarations out of the first
gradient/secondaryVisible blocks so both the createBarGradient call and the
secondaryVisible branch use the same variables; ensure splitHere and left/width
calculations remain based on that single splitWidth and remove the duplicated
declarations inside the secondaryVisible block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 563d2de2-3577-43c6-913e-04230246ae74
📒 Files selected for processing (2)
src/components/HeaderOdakyu.tsxsrc/components/LineBoardEast.tsx
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/NumberingIcon.tsx (1)
321-331: NumberingIcon ラッパーのアーキテクチャに関する補足現在の設計では、ほとんどのアイコンは
lineColorprop を使用し、ラッパーがopacity: 0.25を適用してグレースケールを実現しています。一方、NumberingIconOdakyuは内部で色を管理しているため、lineColor="#000"の上書きは効果がありません。
shouldGrayscaleがNumberingIconOdakyuに渡されるようになった今、Odakyu アイコンに対してはラッパーの処理をスキップする選択肢も検討できます:♻️ オプション: Odakyu アイコンの二重効果を回避する場合
const NumberingIcon = (props: Props) => { const { shouldGrayscale } = props; + // NumberingIconOdakyu handles grayscale internally + if (props.shape === MARK_SHAPE.ODAKYU || props.shape === MARK_SHAPE.HAKONE) { + return <NumberingIconOriginal {...props} />; + } if (!shouldGrayscale) { return <NumberingIconOriginal {...props} />; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/NumberingIcon.tsx` around lines 321 - 331, The wrapper applies a grayscale via styles.pass and overrides lineColor, but NumberingIconOdakyu manages its own colors so lineColor="#000" is ineffective and the wrapper causes a double visual effect; update NumberingIcon to detect when the underlying icon is NumberingIconOdakyu (e.g., via a prop like props.variant/name/component check) and when shouldGrayscale is true and the icon is Odakyu, skip the wrapper and return NumberingIconOriginal {...props} (without passing lineColor and without wrapping in styles.pass); otherwise keep the existing behavior of returning the wrapped icon with lineColor="#000".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/NumberingIcon.tsx`:
- Around line 321-331: The wrapper applies a grayscale via styles.pass and
overrides lineColor, but NumberingIconOdakyu manages its own colors so
lineColor="#000" is ineffective and the wrapper causes a double visual effect;
update NumberingIcon to detect when the underlying icon is NumberingIconOdakyu
(e.g., via a prop like props.variant/name/component check) and when
shouldGrayscale is true and the icon is Odakyu, skip the wrapper and return
NumberingIconOriginal {...props} (without passing lineColor and without wrapping
in styles.pass); otherwise keep the existing behavior of returning the wrapped
icon with lineColor="#000".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 03ee25d9-8093-4db2-8600-f26d77f5a505
📒 Files selected for processing (3)
src/components/LineBoardEast.tsxsrc/components/NumberingIcon.tsxsrc/components/NumberingIconOdakyu.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/LineBoardEast.tsx
Summary
LineBoardEastのバーがchevron位置まで灰色になるように修正isSplitAtCurrentStationのcurrentStationIndex !== 0条件を削除し、最初の駅(index 0)でもバー分割が有効にMath.abs(barLeft) + dotCenterOffsetでドット中心までの距離を灰色幅として使用barWidth - splitWidthにして、分割後の残り領域を正しくカバーTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
リリースノート