Conversation
|
Hvordan ligger du an med denne? Du kan gjerne fullføre PR'en. |
There was a problem hiding this comment.
🟡 Changes recommended
There are confirmed functional issues in session detail loading and timeline grouping/progress calculations that can lead to incorrect UI states or misleading presentation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR implements a major Android UI/navigation redesign aligned with a “Neural Expressive / Material 3 Expressive” direction, centered around making the timeline the primary surface and treating programme year as global context (replacing the old Archive-first navigation).
Changes:
- Removes bottom navigation and introduces an app-bar year picker (bottom sheet) to switch between current and historical programmes.
- Refactors timeline state to be year-aware and derives available day tabs from the selected programme.
- Refreshes Material 3 theming with JavaZone semantic color tokens and a denser typography scale; adds a GitHub Action to compile the Android app on PRs.
File summaries
| File | Description |
|---|---|
| docs/design/neural-expressive-makeover.md | Adds a design spec describing goals, IA, and acceptance criteria for the makeover. |
| app/src/main/java/com/olavbg/javazone/ui/timeline/TimelineViewModel.kt | Introduces selected year state, historical loading, and derived day options for the timeline. |
| app/src/main/java/com/olavbg/javazone/ui/timeline/TimelineScreen.kt | Rebuilds the timeline UI with compact app bar, search/filter affordances, day tabs, and updated session cards. |
| app/src/main/java/com/olavbg/javazone/ui/theme/Type.kt | Defines a compact, high-contrast typography scale for dense conference content. |
| app/src/main/java/com/olavbg/javazone/ui/theme/Theme.kt | Updates theme color schemes and removes dynamic-color usage to preserve JavaZone identity. |
| app/src/main/java/com/olavbg/javazone/ui/theme/Color.kt | Introduces semantic JavaZone color tokens and keeps legacy aliases for incremental migration. |
| app/src/main/java/com/olavbg/javazone/ui/navigation/NavDestinations.kt | Updates routing to preserve year in SessionDetail; removes Archive destination. |
| app/src/main/java/com/olavbg/javazone/ui/JavaZoneApp.kt | Updates navigation flow: timeline → year picker sheet; session detail now carries year. |
| app/src/main/java/com/olavbg/javazone/ui/detail/SessionDetailScreen.kt | Makes session details year-aware and disables favorites for archived years. |
| app/src/main/java/com/olavbg/javazone/ui/components/YearPicker.kt | Adds a bottom-sheet year picker component with radio semantics. |
| .github/workflows/android-build.yml | Adds CI workflow to build the debug APK on PRs / manual dispatch. |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| LaunchedEffect(sessionId, year) { | ||
| loading = true | ||
| session = if (isCurrentYear) repository.getSessions().first().find { it.id == sessionId } | ||
| else repository.getArchiveSessions(year).find { it.id == sessionId } | ||
| loading = false | ||
| } |
| private fun formatTime(zulu: String): String = try { java.time.Instant.parse(zulu).atZone(java.time.ZoneId.of("Europe/Oslo")).toLocalTime().toString().take(5) } catch (_: Exception) { "--:--" } | ||
| private fun isActive(s: Session, now: Instant) = try { now >= Instant.parse(s.startTimeZulu) && now < Instant.parse(s.endTimeZulu) } catch (_: Exception) { false } | ||
| private fun isPast(s: Session, now: Instant) = try { now > Instant.parse(s.endTimeZulu) } catch (_: Exception) { false } | ||
| private fun progress(s: Session, now: Instant): Float = try { val a=Instant.parse(s.startTimeZulu); val b=Instant.parse(s.endTimeZulu); (Duration.between(a, now).toMillis().toFloat()/Duration.between(a,b).toMillis()).coerceIn(0f,1f) } catch (_: Exception) { 0f } |
| val groupedSessions = sessions.map { list -> list.sortedWith(compareBy<Session> { it.startTimeZulu }.thenBy { extractRoomNumber(it.room) }.thenBy { it.room }).groupBy { formatTime(it.startTimeZulu) } } | ||
| .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5000), emptyMap()) |
| data class SessionDetail( | ||
| val sessionId: String, | ||
| val year: Int = 2026, | ||
| ) : NavDestination |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b0ee7d094
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| LaunchedEffect(offset) { simulatedTime = Instant.now().plusMillis(offset) } | ||
| LaunchedEffect(sessionId, year) { | ||
| loading = true | ||
| session = if (isCurrentYear) repository.getSessions().first().find { it.id == sessionId } |
There was a problem hiding this comment.
For current-year detail views, first() captures the Room database's initial snapshot and then stops observing it. A deep link opened before the timeline refresh populates an empty database will therefore remain on “not found”, and favorite writes are never reflected in session, so the heart does not update and a second press still attempts to favorite rather than unfavorite. Continue collecting the session flow as before instead of taking a one-time value.
Useful? React with 👍 / 👎.
|
|
||
| LaunchedEffect(offset) { | ||
| simulatedTime = Instant.now().plusMillis(offset) | ||
| LaunchedEffect(offset) { simulatedTime = Instant.now().plusMillis(offset) } |
There was a problem hiding this comment.
When a user opens a session detail, backgrounds the app through a schedule transition, and returns without changing the simulated-time setting, simulatedTime remains the instant from when the screen was first composed. The live/upcoming/finished banner and remaining-minute text can consequently remain stale indefinitely; restore the resume observer or update this state from a ticking/current-time flow.
Useful? React with 👍 / 👎.
| _selectedFormat.value = null; _selectedLanguage.value = null; _selectedRoom.value = null; _filterSpeaker.value = null; _searchQuery.value = ""; hasScrolledForContext = false | ||
| if (year != CURRENT_YEAR) loadHistoricalYear(year) | ||
| } | ||
| private fun loadHistoricalYear(year: Int) { viewModelScope.launch { _isLoading.value = true; _yearSessions.value = repository.getArchiveSessions(year); _isLoading.value = false } } |
There was a problem hiding this comment.
Selecting one historical year and then another before the first API call completes starts two independent coroutines that share _yearSessions. If the older request completes last, it overwrites the newer result while _selectedYear still names the second year, so the timeline can permanently show the wrong programme under that year label. Cancel the prior load or retain results by year and only publish the response that matches the current selection.
Useful? React with 👍 / 👎.
|
Ga ikke ønsket forbedring i design. |
Overview
This PR delivers the JavaZone Android UI makeover around a compact, developer-focused conference experience inspired by Google's Neural Expressive and Material 3 Expressive direction.
Design goals
JavaZone <year>app-bar control.Implemented
NÅstate.Verification status
The branch is intentionally still a draft. GitHub currently reports no workflow runs for the latest commit, so the final Android build/test verification still needs to be run in CI or locally before marking this ready for review.