[feat] #50,53,54 전략 카드 DnD, 대시보드 DnD, accounts 리다이렉트, 모바일 카드뷰 기본값#91
Closed
fray-cloud wants to merge 0 commit intodevfrom
Closed
[feat] #50,53,54 전략 카드 DnD, 대시보드 DnD, accounts 리다이렉트, 모바일 카드뷰 기본값#91fray-cloud wants to merge 0 commit intodevfrom
fray-cloud wants to merge 0 commit intodevfrom
Conversation
Contributor
Reviewer's GuideImplements drag-and-drop reordering for strategy cards and a customizable dashboard, introduces mobile-first card views for Markets and Portfolio, adds a dashboard entry point to navigation, and replaces the /accounts page with a redirect to /settings while wiring necessary API and dependency changes. Sequence diagram for strategy card drag-and-drop reordersequenceDiagram
actor User
participant StrategiesPage
participant DndContext
participant SortableStrategyCard
participant ApiClient
participant Backend
User->>SortableStrategyCard: drag strategy card
SortableStrategyCard->>DndContext: emit drag events
DndContext->>StrategiesPage: onDragEnd(event)
activate StrategiesPage
StrategiesPage->>StrategiesPage: handleDragEnd(event)
StrategiesPage->>StrategiesPage: compute oldIndex,newIndex
StrategiesPage->>StrategiesPage: newOrder = arrayMove(sortedStrategies)
StrategiesPage->>StrategiesPage: setLocalOrder(newOrder ids)
StrategiesPage->>ApiClient: reorderStrategies(orders)
deactivate StrategiesPage
activate ApiClient
ApiClient->>Backend: PATCH /strategies/reorder { orders }
Backend-->>ApiClient: 200 OK
deactivate ApiClient
activate StrategiesPage
ApiClient-->>StrategiesPage: onSuccess callback
StrategiesPage->>StrategiesPage: invalidateQueries(strategies)
StrategiesPage->>Backend: refetch strategies
Backend-->>StrategiesPage: strategies with updated order
StrategiesPage->>StrategiesPage: sortedStrategies recomputed
deactivate StrategiesPage
Sequence diagram for dashboard widget drag-and-drop and persistencesequenceDiagram
actor User
participant DashboardPage
participant DndContext
participant SortableWidget
participant LocalStorage
User->>SortableWidget: drag widget handle
SortableWidget->>DndContext: drag events
DndContext->>DashboardPage: onDragEnd(event)
activate DashboardPage
DashboardPage->>DashboardPage: handleDragEnd(event)
DashboardPage->>DashboardPage: arrayMove(widgetOrder)
DashboardPage->>DashboardPage: setWidgetOrder(newOrder)
DashboardPage->>LocalStorage: setItem(STORAGE_KEY,newOrder)
deactivate DashboardPage
rect rgb(240,240,240)
Note over DashboardPage,LocalStorage: Next page load
end
DashboardPage->>LocalStorage: getItem(STORAGE_KEY)
LocalStorage-->>DashboardPage: serialized order
DashboardPage->>DashboardPage: loadWidgetOrder() with fallback
Class diagram for strategy drag-and-drop orderingclassDiagram
class StrategyItem {
+string id
+string name
+string symbol
+boolean enabled
+number intervalSeconds
+string candleInterval
+number order
+string createdAt
+string updatedAt
}
class ApiClient {
+Promise~StrategyItem[]~ getStrategies()
+Promise~void~ toggleStrategy(string id)
+Promise~void~ deleteStrategy(string id)
+Promise~void~ reorderStrategies(OrderUpdate[] orders)
}
class OrderUpdate {
+string id
+number order
}
class StrategiesPage {
-StrategyItem[] strategies
-string[] localOrder
-StrategyItem[] sortedStrategies
+void handleDragEnd(DragEndEvent event)
+void setLocalOrder(string[] nextOrder)
}
class SortableStrategyCard {
+StrategyItem strategy
+void onToggle()
+void onDelete()
}
class DndContext {
+void onDragEnd(DragEndEvent event)
}
class SortableContext {
+string[] items
}
class DragEndEvent {
+string activeId
+string overId
}
StrategiesPage --> StrategyItem : uses
StrategiesPage --> ApiClient : calls
StrategiesPage --> DndContext : configures
DndContext --> StrategiesPage : onDragEnd callback
StrategiesPage --> SortableContext : wraps_list
SortableContext --> SortableStrategyCard : provides_sorting
SortableStrategyCard --> StrategyItem : renders
ApiClient --> OrderUpdate : uses
Class diagram for dashboard widgets and mobile card viewsclassDiagram
class DashboardPage {
-WidgetId[] widgetOrder
+void handleDragEnd(DragEndEvent event)
+WidgetId[] loadWidgetOrder()
+void saveWidgetOrder(WidgetId[] order)
}
class SortableWidget {
+WidgetId id
}
class PortfolioWidget {
+void render()
}
class StrategiesWidget {
+void render()
}
class OrdersWidget {
+void render()
}
class MarketsWidget {
+void render()
}
class WidgetId {
<<enumeration>>
portfolio
strategies
orders
markets
}
class TickerCardList {
+Ticker[] tickers
+void setFilter(string value)
}
class SwipableTickerCard {
+Ticker ticker
+void onQuickOrder(Ticker ticker)
}
class AssetCardList {
+PortfolioAsset[] assets
+void setSearchText(string value)
}
class AssetCard {
+PortfolioAsset asset
}
class Ticker {
+string exchange
+string symbol
+string price
+string changePercent24h
+string high24h
+string low24h
}
class PortfolioAsset {
+string exchange
+string currency
+number quantity
+number valueKrw
+number avgCost
+number currentPrice
+number pnl
}
class QuickOrderPanel {
+Ticker ticker
+void onClose()
}
DashboardPage --> SortableWidget : renders
DashboardPage --> WidgetId : manages_order
SortableWidget --> WidgetId : identifies
SortableWidget --> PortfolioWidget : may_render
SortableWidget --> StrategiesWidget : may_render
SortableWidget --> OrdersWidget : may_render
SortableWidget --> MarketsWidget : may_render
TickerCardList --> SwipableTickerCard : renders_list
TickerCardList --> Ticker : uses
SwipableTickerCard --> Ticker : displays
TickerCardList --> QuickOrderPanel : controls
AssetCardList --> AssetCard : renders_list
AssetCardList --> PortfolioAsset : filters
AssetCard --> PortfolioAsset : displays
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- Several user-facing strings are hard-coded in the new components (e.g., dashboard link label, dashboard widget titles, TickerCardList messages like
Order,← Swipe left for quick order, and the no-results text), so it would be better to move these into next-intl translations for consistency with the rest of the app. - The DnD implementations for strategy cards and dashboard widgets share very similar patterns (sensors setup, useSortable, drag handle UI, arrayMove logic); consider extracting a small reusable sortable wrapper/hook to reduce duplication and make future DnD behavior changes easier to apply across both areas.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Several user-facing strings are hard-coded in the new components (e.g., dashboard link label, dashboard widget titles, TickerCardList messages like `Order`, `← Swipe left for quick order`, and the no-results text), so it would be better to move these into next-intl translations for consistency with the rest of the app.
- The DnD implementations for strategy cards and dashboard widgets share very similar patterns (sensors setup, useSortable, drag handle UI, arrayMove logic); consider extracting a small reusable sortable wrapper/hook to reduce duplication and make future DnD behavior changes easier to apply across both areas.
## Individual Comments
### Comment 1
<location path="apps/web/src/app/strategies/page.tsx" line_range="102-106" />
<code_context>
+ onSuccess: () => queryClient.invalidateQueries({ queryKey: ['strategies'] }),
+ });
+
+ const sortedStrategies = useMemo(() => {
+ if (!localOrder) return [...strategies].sort((a, b) => a.order - b.order);
+ return localOrder
+ .map((id) => strategies.find((s) => s.id === id))
+ .filter(Boolean) as StrategyItem[];
+ }, [strategies, localOrder]);
+
</code_context>
<issue_to_address>
**issue (bug_risk):** New strategies coming from the server can be dropped when a local drag order is active.
Because `sortedStrategies` only includes IDs from `localOrder` once it’s set, any new items from `strategies` that weren’t in the drag at the time will never show up, and removed items can linger as long as their IDs remain in `localOrder`.
Consider either (1) always including strategies whose IDs aren’t in `localOrder` (e.g. append them using backend `order`), and/or (2) reconciling `localOrder` whenever `strategies` changes (e.g. filter out missing IDs and append new ones in a `useEffect`) so the UI stays aligned with server state while preserving drag order.
</issue_to_address>
### Comment 2
<location path="apps/web/src/components/markets/ticker-card-list.tsx" line_range="56-59" />
<code_context>
+
+const SWIPE_THRESHOLD = 60;
+
+function SwipableTickerCard({
+ ticker,
+ onQuickOrder,
+}: {
+ ticker: Ticker;
+ onQuickOrder: (ticker: Ticker) => void;
</code_context>
<issue_to_address>
**suggestion (performance):** Per-card hooks for exchange rate and base currency can be lifted to reduce redundant work.
`SwipableTickerCard` calls `useExchangeRate` and `useBaseCurrency` for every card, even though these values are global. On large lists this adds unnecessary hook work and re-renders.
Consider moving these hooks to `TickerCardList` and passing `krwPerUsd` and `baseCurrency` as props to each card to avoid repeated calls and improve list performance.
Suggested implementation:
```typescript
const SWIPE_THRESHOLD = 60;
type SwipableTickerCardProps = {
ticker: Ticker;
onQuickOrder: (ticker: Ticker) => void;
krwPerUsd: number | null;
baseCurrency: CurrencyCode; // adjust to the existing currency type if different
};
function SwipableTickerCard({
ticker,
onQuickOrder,
krwPerUsd,
baseCurrency,
}: SwipableTickerCardProps) {
const touchStartX = useRef<number | null>(null);
```
```typescript
function TickerCardList({
tickers,
onQuickOrder,
}: {
tickers: Ticker[];
onQuickOrder: (ticker: Ticker) => void;
}) {
const { krwPerUsd } = useExchangeRate();
const { currency: baseCurrency } = useBaseCurrency();
```
```typescript
{tickers.map((ticker) => (
<SwipableTickerCard
key={ticker.symbol}
ticker={ticker}
onQuickOrder={onQuickOrder}
krwPerUsd={krwPerUsd}
baseCurrency={baseCurrency}
/>
))}
```
1. Ensure `CurrencyCode` (used in `SwipableTickerCardProps`) is an existing type in this file or imported from your shared types. If the codebase uses a different type (e.g. `Currency`, `BaseCurrency`, or just `string`), update the `baseCurrency` type accordingly.
2. If `TickerCardList` currently has a different props shape or additional props (e.g. filters, layout options), merge the added hooks into the existing function body without removing other logic.
3. If multiple list components render `SwipableTickerCard` from this file, each must now pass `krwPerUsd` and `baseCurrency` (or delegate to a shared list component that does).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+102
to
+106
| const sortedStrategies = useMemo(() => { | ||
| if (!localOrder) return [...strategies].sort((a, b) => a.order - b.order); | ||
| return localOrder | ||
| .map((id) => strategies.find((s) => s.id === id)) | ||
| .filter(Boolean) as StrategyItem[]; |
Contributor
There was a problem hiding this comment.
issue (bug_risk): New strategies coming from the server can be dropped when a local drag order is active.
Because sortedStrategies only includes IDs from localOrder once it’s set, any new items from strategies that weren’t in the drag at the time will never show up, and removed items can linger as long as their IDs remain in localOrder.
Consider either (1) always including strategies whose IDs aren’t in localOrder (e.g. append them using backend order), and/or (2) reconciling localOrder whenever strategies changes (e.g. filter out missing IDs and append new ones in a useEffect) so the UI stays aligned with server state while preserving drag order.
Comment on lines
+56
to
+59
| function SwipableTickerCard({ | ||
| ticker, | ||
| onQuickOrder, | ||
| }: { |
Contributor
There was a problem hiding this comment.
suggestion (performance): Per-card hooks for exchange rate and base currency can be lifted to reduce redundant work.
SwipableTickerCard calls useExchangeRate and useBaseCurrency for every card, even though these values are global. On large lists this adds unnecessary hook work and re-renders.
Consider moving these hooks to TickerCardList and passing krwPerUsd and baseCurrency as props to each card to avoid repeated calls and improve list performance.
Suggested implementation:
const SWIPE_THRESHOLD = 60;
type SwipableTickerCardProps = {
ticker: Ticker;
onQuickOrder: (ticker: Ticker) => void;
krwPerUsd: number | null;
baseCurrency: CurrencyCode; // adjust to the existing currency type if different
};
function SwipableTickerCard({
ticker,
onQuickOrder,
krwPerUsd,
baseCurrency,
}: SwipableTickerCardProps) {
const touchStartX = useRef<number | null>(null);function TickerCardList({
tickers,
onQuickOrder,
}: {
tickers: Ticker[];
onQuickOrder: (ticker: Ticker) => void;
}) {
const { krwPerUsd } = useExchangeRate();
const { currency: baseCurrency } = useBaseCurrency(); {tickers.map((ticker) => (
<SwipableTickerCard
key={ticker.symbol}
ticker={ticker}
onQuickOrder={onQuickOrder}
krwPerUsd={krwPerUsd}
baseCurrency={baseCurrency}
/>
))}- Ensure
CurrencyCode(used inSwipableTickerCardProps) is an existing type in this file or imported from your shared types. If the codebase uses a different type (e.g.Currency,BaseCurrency, or juststring), update thebaseCurrencytype accordingly. - If
TickerCardListcurrently has a different props shape or additional props (e.g. filters, layout options), merge the added hooks into the existing function body without removing other logic. - If multiple list components render
SwipableTickerCardfrom this file, each must now passkrwPerUsdandbaseCurrency(or delegate to a shared list component that does).
Owner
Author
|
모든 커밋이 이미 dev에 반영되어 있음 (squash merge로 포함됨). 브랜치를 삭제하고 PR을 닫습니다. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
변경 사항
feat(web): 모바일 카드뷰 기본값 — Markets/Portfolio 페이지 (PRO-54 연동)feat(web):/accounts→/settings?tab=accounts리다이렉트 (PRO-54)feat(web): 전략 카드 DnD 순서 변경 구현 (PRO-50)feat(web): 대시보드 위젯 DnD 레이아웃 구현 (PRO-53)fix(web): Sourcery 리뷰 반영 —useMemo로sortedStrategies최적화, localStorage SSR guard (PRO-50)관련 이슈
Closes #85
(covers PRO-50, PRO-53, PRO-54)
테스트 방법
/accounts접근 →/settings?tab=accounts로 리다이렉트 확인스크린샷 (UI 변경 시)
전략 카드 DnD, 대시보드 위젯 DnD 포함.
Summary by Sourcery
Introduce drag-and-drop reordering for strategies and a customizable dashboard layout, update navigation and mobile defaults, and add an accounts settings redirect.
New Features:
Bug Fixes:
Enhancements:
Build: