-
Notifications
You must be signed in to change notification settings - Fork 108
[MBL-19500][Student] Add expand/collapse functionality to conference groups #3373
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
[MBL-19500][Student] Add expand/collapse functionality to conference groups #3373
Conversation
…e groups Add expand/collapse functionality to "New conferences" and "Concluded conferences" sections on the Conferences list page. Changes: - Add expand/collapse state tracking in ConferenceListModel - Add HeaderClicked event handling - Add expand icon to conference section headers - Implement header click handlers to toggle section visibility - Add icon rotation animation based on expanded state Technical details: - Both sections start expanded by default - Sections expand/collapse independently - Uses existing Mobius architecture pattern - Added ic_expand_more drawable and a11y_expand_collapse string refs: MBL-19500 affects: Student release note: Added expand/collapse functionality to conference list sections 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
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 Review Summary
This PR adds collapsible/expandable sections to the conference list, allowing users to expand or collapse "New Conferences" and "Concluded Conferences" headers. Overall, the implementation follows a clean MVI architecture pattern and the functionality is well-structured.
✅ Positive Aspects
- Clean MVI architecture with proper separation of concerns (Model, Update, Presenter, View)
- State management is well-handled with expansion state tracked in the model
- Good use of Kotlin data classes and sealed classes
- Accessibility content description added for the expand/collapse icon
- The UI changes are minimal and focused on the feature requirements
Issues Found
- Code Style: Fully qualified package names used instead of imports in multiple files (ConferenceListAdapter.kt:33, ConferenceListView.kt:88, ConferenceListViewState.kt:30-34)
- Theme Support: Hardcoded color
#2D3B45in ic_expand_more.xml:8 should use color resource for proper theme support - File Format: Missing newline at end of ic_expand_more.xml:9
- UX Enhancement: Icon rotation could benefit from animation for smoother visual feedback (ConferenceListAdapter.kt:67)
Additional Observations
Architecture: The Mobius pattern implementation is solid, with events flowing properly through the system (HeaderClicked event → Update → Model → Presenter → View).
State Persistence: The expansion states (isNewConferencesExpanded, isConcludedConferencesExpanded) default to true, which provides a good user experience. However, consider whether these states should be persisted across app restarts for better UX continuity.
Testing Considerations:
- Unit tests should verify the toggle behavior in ConferenceListUpdate
- UI tests should verify the expand/collapse animation and content visibility
- Consider testing edge cases like rapid clicking on headers
Performance: The implementation efficiently filters items in the presenter based on expansion state, avoiding unnecessary view binding for collapsed sections.
Overall, this is a well-implemented feature that just needs minor cleanup for code style consistency and theme support. Great work!
.../java/com/instructure/student/mobius/conferences/conference_list/ui/ConferenceListAdapter.kt
Outdated
Show resolved
Hide resolved
...ain/java/com/instructure/student/mobius/conferences/conference_list/ui/ConferenceListView.kt
Outdated
Show resolved
Hide resolved
.../java/com/instructure/student/mobius/conferences/conference_list/ui/ConferenceListAdapter.kt
Outdated
Show resolved
Hide resolved
.../java/com/instructure/student/mobius/conferences/conference_list/ui/ConferenceListAdapter.kt
Outdated
Show resolved
Hide resolved
.../java/com/instructure/student/mobius/conferences/conference_list/ui/ConferenceListAdapter.kt
Outdated
Show resolved
Hide resolved
...ava/com/instructure/student/mobius/conferences/conference_list/ui/ConferenceListViewState.kt
Outdated
Show resolved
Hide resolved
...n/java/com/instructure/student/mobius/conferences/conference_list/ConferenceListPresenter.kt
Show resolved
Hide resolved
...main/java/com/instructure/student/mobius/conferences/conference_list/ConferenceListUpdate.kt
Show resolved
Hide resolved
Unit Test Updates:
- Updated ConferenceListPresenterTest to reflect changes to ConferenceHeader data class
- Added headerType and isExpanded parameters to existing header assertions
- Added 3 new test cases for collapse functionality:
* Returns only header when new conferences section is collapsed
* Returns only header when concluded conferences section is collapsed
* Sections can be collapsed independently
- All 12 tests passing
- Updated ConferenceListUpdateTest to test HeaderClicked event handling
- Added 5 new test cases for header toggle behavior:
* HeaderClicked with NEW_CONFERENCES toggles isNewConferencesExpanded (both directions)
* HeaderClicked with CONCLUDED_CONFERENCES toggles isConcludedConferencesExpanded (both directions)
* HeaderClicked only toggles the targeted section
- All 11 tests passing
Linter Improvements:
- Applied automatic import organization
- Changed icon rotation to use animate() for smooth animation (200ms duration)
- Updated header layout to use existing ic_expand drawable with textDark tint
- Added explicit import for ConferenceHeaderType where needed
📊 Code Coverage Report
|
adamNagy56
left a comment
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.
QA +1
Summary
This PR adds expand/collapse functionality to the "New conferences" and "Concluded conferences" sections on the Conferences list page. Users can now tap on section headers to expand or collapse the list of conferences in that section.
Changes
isNewConferencesExpanded,isConcludedConferencesExpanded)HeaderClickedevent andConferenceHeaderTypeenum to handle section header interactionsTechnical Details
ic_expand_more.xmldrawable anda11y_expand_collapseaccessibility stringTest Plan
Screenshots
refs: MBL-19500
affects: Student
release note: Added expand/collapse functionality to conference list sections, allowing users to hide or show "New conferences" and "Concluded conferences" groups
Checklist
🤖 Generated with Claude Code