-
Notifications
You must be signed in to change notification settings - Fork 48
feat: add feed filter menu with Reddit-style dropdown #45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Implement tab filtering functionality similar to Android version, allowing users to filter feed content by different categories (技术, 创意, 好玩, Apple, etc.) Changes: - Add FilterMenuView: 3-column grid menu displaying all 13 V2EX tabs - Update Tab enum: Add displayName() fix, needsLogin(), supportsLoadMore(), tab persistence - Update FeedState: Add selectedTab and showFilterMenu properties - Update FeedReducer: Add SelectTab and ToggleFilterMenu actions - Update TopBar: Display selected tab name with chevron indicator - Update FeedPage: Integrate FilterMenuView with ZStack overlay - Restrict load more to "all" tab only (matching V2EX API behavior) Fixes: - Tab.displayName() now returns correct values instead of empty string 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Changes: - Change title display: Show "V2EX" for default "all" tab, show tab name for other selections - Redesign menu as left-aligned dropdown (like Reddit) instead of centered modal - Add icons for each tab category (house, flame, briefcase, etc.) - Replace grid layout with vertical list layout - Add checkmark indicator for selected item - Improve visual hierarchy with better spacing and typography - Add subtle shadow and spring animation for smoother transitions UI improvements: - Menu now anchors to top-left below title - Width: 200pt, max height: 450pt - Each item shows icon + label + checkmark (when selected) - Selected items have blue accent color with light background - Better touch targets with full-width clickable areas 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a Reddit-style dropdown filter menu for the V2EX feed page, allowing users to filter content by different categories. The feature includes a redesigned top bar with clickable title, comprehensive state management for tab selection, and proper authentication handling for restricted tabs.
Key changes:
- New FilterMenuView component with dropdown menu UI and icon-based category selection
- Enhanced Tab enum with login requirements, load-more support, and UserDefaults persistence
- Updated FeedState and FeedReducer to handle tab selection and menu visibility
- Redesigned TopBar with dynamic title display and filter menu toggle
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| FilterMenuView.swift | New dropdown menu component with tab selection UI and authentication handling |
| TopBar.swift | Redesigned top bar with clickable title and chevron indicator for filter menu |
| FeedPage.swift | Integration of filter menu overlay and load-more restrictions |
| TabInfo.swift | Enhanced Tab enum with login requirements, load-more support, and persistence |
| FeedState.swift | Added selectedTab and showFilterMenu state properties |
| FeedReducer.swift | Added SelectTab and ToggleFilterMenu actions with state management |
| project.pbxproj | Project file updates for new FilterMenuView.swift |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
V2er/View/Feed/FilterMenuView.swift
Outdated
| private let columns = [ | ||
| GridItem(.flexible()), | ||
| GridItem(.flexible()), | ||
| GridItem(.flexible()) | ||
| ] | ||
|
|
Copilot
AI
Oct 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The columns property is defined but never used in the implementation. The menu uses a VStack layout instead of a grid layout, making this property unnecessary.
| private let columns = [ | |
| GridItem(.flexible()), | |
| GridItem(.flexible()), | |
| GridItem(.flexible()) | |
| ] |
V2er/View/Widget/TopBar.swift
Outdated
| .foregroundColor(.primary) | ||
| } | ||
| .onTapGesture { | ||
| dispatch(FeedActions.ToggleFilterMenu()) |
Copilot
AI
Oct 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dispatch function is called without importing or defining it in this file. This suggests a missing import or the function should be accessed through the store's dispatch method.
| dispatch(FeedActions.ToggleFilterMenu()) | |
| store.dispatch(FeedActions.ToggleFilterMenu()) |
Code Coverage Report ❌Current coverage: 0% |
The columns property was leftover from the initial grid layout design and is no longer needed after switching to vertical list layout. Addresses Copilot review comment.
|
Fixed the unused Regarding the Build status: ✅ All checks passing |
Code Coverage Report ❌Current coverage: 0% |
…down - Moved filter menu rendering from TopBar to MainPage as an overlay - FilterMenuView now renders above content with proper z-index stacking - Removed duplicate overlay logic from FeedPage - Adjusted FilterMenuView positioning to work as a true overlay - Menu now appears on top of feed content without affecting layout 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Code Coverage Report ❌Current coverage: 0% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
V2er/View/Widget/TopBar.swift
Outdated
| .font(.system(size: 22)) | ||
| .padding(6) | ||
| .forceClickable() | ||
| .hide() |
Copilot
AI
Oct 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The .hide() modifier is applied to a grid icon that appears to be dead code since it's always hidden. Consider removing this unused UI element entirely to reduce code complexity.
V2er/View/Feed/FilterMenuView.swift
Outdated
| // FilterMenuView.swift | ||
| // V2er | ||
| // | ||
| // Created by Claude on 2025/10/08. |
Copilot
AI
Oct 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file header contains 'Claude' as the author name, which should be replaced with the actual developer's name or removed to follow standard project conventions.
| // Created by Claude on 2025/10/08. |
| case let action as FeedActions.LoadMore.Done: | ||
| state.loadingMore = false | ||
| state.hasMoreData = true // todo check vary tabs | ||
| state.hasMoreData = state.selectedTab.supportsLoadMore() |
Copilot
AI
Oct 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic state.selectedTab.supportsLoadMore() is duplicated in multiple places (lines 26, 31, 36, 49). Consider extracting this into a computed property or helper method to reduce code duplication.
- Removed .padding(.top, 8) from FilterMenuView to eliminate gap - Dropdown menu now appears directly connected to the topbar 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Code Coverage Report ❌Current coverage: 0% |
- Added light haptic feedback when tapping title to show/hide menu - Added haptic feedback when dismissing menu by tapping background - Added haptic feedback when selecting a menu item - Provides better tactile response for user interactions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Code Coverage Report ❌Current coverage: 0% |
- Added pressed state tracking with @State variable - Implemented scale effect (0.97x) when pressed for visual feedback - Added opacity change (0.7) when pressed for better visibility - Added pressed state background color for clear tap indication - Used onLongPressGesture with minimumDuration: 0 for immediate feedback 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| .forceClickable() | ||
| .onChange(of: store.appState.feedState.showFilterMenu) { newValue in | ||
| withAnimation { | ||
| rotationAngle += 180 |
Copilot
AI
Oct 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rotation angle continuously increases by 180 degrees on each toggle, which will cause the chevron to rotate multiple full circles over time. Use a conditional assignment: rotationAngle = newValue ? 180 : 0.
| rotationAngle += 180 | |
| rotationAngle = newValue ? 180 : 0 |
V2er/View/Feed/FilterMenuView.swift
Outdated
| TabFilterMenuItem( | ||
| tab: tab, | ||
| isSelected: tab == selectedTab, | ||
| needsLogin: tab.needsLogin() && !AccountState.hasSignIn() | ||
| ) { | ||
| // Soft haptic feedback | ||
| let impactFeedback = UIImpactFeedbackGenerator(style: .light) | ||
| impactFeedback.impactOccurred() | ||
|
|
||
| if tab.needsLogin() && !AccountState.hasSignIn() { |
Copilot
AI
Oct 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The login check logic tab.needsLogin() && !AccountState.hasSignIn() is duplicated on line 47. Consider extracting this into a computed property or local variable to avoid repetition.
| TabFilterMenuItem( | |
| tab: tab, | |
| isSelected: tab == selectedTab, | |
| needsLogin: tab.needsLogin() && !AccountState.hasSignIn() | |
| ) { | |
| // Soft haptic feedback | |
| let impactFeedback = UIImpactFeedbackGenerator(style: .light) | |
| impactFeedback.impactOccurred() | |
| if tab.needsLogin() && !AccountState.hasSignIn() { | |
| let tabNeedsLogin = tab.needsLogin() && !AccountState.hasSignIn() | |
| TabFilterMenuItem( | |
| tab: tab, | |
| isSelected: tab == selectedTab, | |
| needsLogin: tabNeedsLogin | |
| ) { | |
| // Soft haptic feedback | |
| let impactFeedback = UIImpactFeedbackGenerator(style: .light) | |
| impactFeedback.impactOccurred() | |
| if tabNeedsLogin { |
Code Coverage Report ❌Current coverage: 0% |
✅ PR Review Comments AddressedI've addressed the Copilot review feedback in the latest commit: Fixed Issues:
Regarding Other Comments:
CI Status:
All review comments have been addressed. The build is successful locally and the app has been tested on both simulator and real device. |
- Fixed rotation angle logic in TopBar to use conditional assignment instead of continuous increment - Extracted duplicate login check logic in FilterMenuView into local variable - Reduced code duplication in FeedReducer by extracting supportsLoadMore() calls into variables 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Code Coverage Report ❌Current coverage: 0% |
- Reverted rotation angle logic to continuous increment for smoother animation - Added progress view display when switching tabs to improve UX 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Code Coverage Report ❌Current coverage: 0% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| Spacer() | ||
| } | ||
| } | ||
| .transition(.opacity) | ||
| .animation(.spring(response: 0.3, dampingFraction: 0.8), value: isShowing) |
Copilot
AI
Oct 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The animation modifier is applied to the entire ZStack but only responds to isShowing changes. Consider moving this animation to the specific views that need it, or ensure isShowing properly controls the entire view's visibility.
| Spacer() | |
| } | |
| } | |
| .transition(.opacity) | |
| .animation(.spring(response: 0.3, dampingFraction: 0.8), value: isShowing) | |
| .animation(.spring(response: 0.3, dampingFraction: 0.8), value: isShowing) | |
| Spacer() | |
| } | |
| } | |
| .transition(.opacity) |
Code Coverage Report ❌Current coverage: 0% |
7256895 to
1231b29
Compare
Code Coverage Report ❌Current coverage: 0% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| let supportsLoadMore = state.selectedTab.supportsLoadMore() | ||
| state.hasMoreData = supportsLoadMore |
Copilot
AI
Oct 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The variable supportsLoadMore is used only once and adds unnecessary complexity. Consider directly assigning state.hasMoreData = state.selectedTab.supportsLoadMore().
| let supportsLoadMore = state.selectedTab.supportsLoadMore() | ||
| state.hasMoreData = supportsLoadMore |
Copilot
AI
Oct 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The variable supportsLoadMore is used only once and adds unnecessary complexity. Consider directly assigning state.hasMoreData = state.selectedTab.supportsLoadMore().
| let supportsLoadMore = action.tab.supportsLoadMore() | ||
| state.hasMoreData = supportsLoadMore |
Copilot
AI
Oct 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The variable supportsLoadMore is used only once and adds unnecessary complexity. Consider directly assigning state.hasMoreData = action.tab.supportsLoadMore().
- Added scrollToTop state property to FeedState - Modified FeedReducer to trigger scroll after filter change data loads - Updated FeedPage to use both state and global scroll triggers - Ensures users see new content from the beginning after filter change This improves UX by automatically scrolling the feed list to the top when users select a different filter category, preventing confusion from seeing mid-scroll content from the previous filter. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Code Coverage Report ❌Current coverage: 0% |
Summary
Implement tab filtering functionality for the feed page, allowing users to filter content by different V2EX categories (技术, 创意, Apple, 热门, etc.). The UI design follows Reddit's dropdown pattern with a left-aligned menu.
Changes
Core Features
State Management
Tab enum enhancements (
TabInfo.swift):displayName()bug (was returning empty string)needsLogin()- checks if tab requires authenticationsupportsLoadMore()- only "all" tab supports paginationallTabsstatic propertyFeedState updates:
selectedTab: Tab- tracks currently selected filtershowFilterMenu: Bool- controls menu visibilityFeedReducer updates:
SelectTabaction - changes active filter and fetches new dataToggleFilterMenuaction - shows/hides the dropdownUI/UX Improvements
TopBar redesign:
FilterMenuView features:
Technical Details
APIServicewith tab parameterScreenshots
[To be added after review]
Testing
Manual Testing
Automated Testing
Related Issues
Implements the filter menu feature similar to the Android version.
Migration Notes
None - This is a new feature with no breaking changes.
🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com