Skip to content
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

ListDetailPaneScaffold #970

Closed
wants to merge 7 commits into from
Closed

Conversation

mmoczkowski
Copy link
Contributor

No description provided.

@mmoczkowski mmoczkowski changed the base branch from main to feature/navigation-suite-scaffold October 3, 2023 16:01
@alexvanyo
Copy link
Contributor

One behavior issue I noticed: on a small screen (without list+detail showing together), if you tap on a topic, then navigate back, then tap on the same topic, the second tap doesn't navigate to the topic again:

Screen_recording_20231017_173006.mp4


fun followTopic(followedTopicId: String, followed: Boolean) {
viewModelScope.launch {
userDataRepository.setTopicIdFollowed(followedTopicId, followed)
}
}

fun onTopicClick(topicId: String) {
viewModelScope.launch {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need for a launch here if we aren't calling a suspending function.

@mmoczkowski mmoczkowski force-pushed the feature/navigation-suite-scaffold branch 5 times, most recently from 544e4db to 816a7b6 Compare November 16, 2023 16:14
Base automatically changed from feature/navigation-suite-scaffold to main November 16, 2023 16:54
Change-Id: I13cca7db13411794e333d34f6edacf594586ef6d
@mmoczkowski mmoczkowski force-pushed the feature/list-detail-pane-scaffold branch from e9c64f3 to dfff806 Compare November 17, 2023 13:03
Change-Id: I4d77c84da8d18cec87f3b7a47603d62bfa9c790d
Change-Id: Ice4996331a1c44e5284839b2cdf2330c737361bf
Change-Id: I04c00490e97e0c41f5cc285df6a62014e6b12e47
Change-Id: I7e3ef3ca9bd01c436784bff0a33208abe3bc4703
Change-Id: I5ca91db033a658450144439e90ea4b880f57b7fd
Change-Id: Ic18da787dc58f2ef617149d97ad42e57da0e9a6a
@jdkoren
Copy link
Contributor

jdkoren commented Feb 16, 2024

One behavior issue I noticed: on a small screen (without list+detail showing together), if you tap on a topic, then navigate back, then tap on the same topic, the second tap doesn't navigate to the topic again:

Good catch. I found a few more bugs with navigation, both on phone and tablet. Repro steps:

  • From For you, tap on a topic chip.
    • Observed: App switched to Interests but shows the list pane (no detail pane, not even on tablet).
    • Expected: Switch to Interests and show the selected topic (detail pane if single pane, list and detail if multi-pane).
  • In this state, tap on For you.
    • Observed: Nothing happens, Interests remains selected.
    • Expected: App switches back to For You.
  • Tap on Saved, app switches to Saved. Now tap on For you.
    • Observed: App switches back to Interests.
    • Expected: App switches to For you.
Screen_recording_20240216_093654.mp4

@jdkoren
Copy link
Contributor

jdkoren commented Mar 7, 2024

Superseded by #1234

@jdkoren jdkoren closed this Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants