Skip to content

Commit b9df6cf

Browse files
lorensrlorensr
committed
Code review
1 parent e87adb5 commit b9df6cf

File tree

12 files changed

+150
-86
lines changed

12 files changed

+150
-86
lines changed

‎app/build.gradle.kts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ dependencies {
4242
implementation("androidx.recyclerview:recyclerview:1.1.0")
4343
implementation("androidx.navigation:navigation-fragment-ktx:2.3.0")
4444
implementation("androidx.navigation:navigation-ui-ktx:2.3.0")
45-
implementation("com.google.android.material:material:1.1.0")
45+
implementation("com.google.android.material:material:1.3.0-alpha01")
4646
implementation("com.apollographql.apollo:apollo-runtime:2.2.2")
4747
implementation("com.apollographql.apollo:apollo-coroutines-support:2.2.2")
4848
}

‎app/src/main/java/guide/graphql/toc/ChaptersAdapter.kt

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
package guide.graphql.toc
22

33
import android.view.LayoutInflater
4+
import android.view.View
45
import android.view.ViewGroup
56
import androidx.recyclerview.widget.RecyclerView
67
import guide.graphql.toc.databinding.ChapterBinding
78

89
class ChaptersAdapter(
9-
private val chapters: List<ChaptersQuery.Chapter>
10+
private val chapters: List<ChaptersQuery.Chapter>,
11+
private val onItemClicked: ((ChaptersQuery.Chapter) -> Unit)
1012
) :
1113
RecyclerView.Adapter<ChaptersAdapter.ViewHolder>() {
1214

@@ -21,18 +23,22 @@ class ChaptersAdapter(
2123
return ViewHolder(binding)
2224
}
2325

24-
var onItemClicked: ((ChaptersQuery.Chapter) -> Unit)? = null
25-
2626
override fun onBindViewHolder(holder: ViewHolder, position: Int) {
2727
val chapter = chapters[position]
2828
val header =
2929
if (chapter.number == null) chapter.title else "Chapter ${chapter.number.toInt()}"
3030

3131
holder.binding.chapterHeader.text = header
32-
holder.binding.chapterSubheader.text = if (chapter.number == null) "" else chapter.title
32+
if (chapter.number == null) {
33+
holder.binding.chapterSubheader.visibility = View.GONE
34+
35+
} else {
36+
holder.binding.chapterSubheader.text = chapter.title
37+
holder.binding.chapterSubheader.visibility = View.VISIBLE
38+
}
3339

3440
holder.binding.root.setOnClickListener {
35-
onItemClicked?.invoke(chapter)
41+
onItemClicked.invoke(chapter)
3642
}
3743
}
3844
}

‎app/src/main/java/guide/graphql/toc/ChaptersFragment.kt

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,16 @@ import android.util.Log
55
import android.view.LayoutInflater
66
import android.view.View
77
import android.view.ViewGroup
8+
import androidx.appcompat.app.AppCompatActivity
89
import androidx.fragment.app.Fragment
910
import androidx.lifecycle.lifecycleScope
1011
import androidx.navigation.fragment.findNavController
12+
import androidx.recyclerview.widget.DividerItemDecoration
1113
import androidx.recyclerview.widget.LinearLayoutManager
14+
import androidx.recyclerview.widget.RecyclerView
1215
import com.apollographql.apollo.coroutines.toDeferred
1316
import com.apollographql.apollo.exception.ApolloException
17+
import com.google.android.material.transition.MaterialSharedAxis
1418
import guide.graphql.toc.databinding.ChaptersFragmentBinding
1519

1620
class ChaptersFragment : Fragment() {
@@ -25,9 +29,22 @@ class ChaptersFragment : Fragment() {
2529
return binding.root
2630
}
2731

32+
override fun onCreate(savedInstanceState: Bundle?) {
33+
super.onCreate(savedInstanceState)
34+
35+
36+
val backward = MaterialSharedAxis(MaterialSharedAxis.Z, false)
37+
reenterTransition = backward
38+
39+
val forward = MaterialSharedAxis(MaterialSharedAxis.Z, true)
40+
exitTransition = forward
41+
}
42+
2843
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
2944
super.onViewCreated(view, savedInstanceState)
3045

46+
(requireActivity() as AppCompatActivity).setSupportActionBar(binding.bookHeader)
47+
3148
lifecycleScope.launchWhenResumed {
3249
val response = try {
3350
apolloClient.query(
@@ -38,23 +55,26 @@ class ChaptersFragment : Fragment() {
3855
return@launchWhenResumed
3956
}
4057

41-
val chapters = response.data?.chapters
42-
if (chapters == null || response.hasErrors()) {
58+
if (response.hasErrors()) {
4359
return@launchWhenResumed
4460
}
4561

46-
val adapter =
47-
ChaptersAdapter(chapters)
48-
binding.chapters.layoutManager = LinearLayoutManager(requireContext())
49-
binding.chapters.adapter = adapter
50-
51-
adapter.onItemClicked = { chapter ->
52-
findNavController().navigate(
53-
ChaptersFragmentDirections.viewSections(
54-
chapterId = chapter.id
55-
)
56-
)
62+
response.data?.chapters?.let { chapters ->
63+
val adapter =
64+
ChaptersAdapter(chapters) { chapter ->
65+
findNavController().navigate(
66+
ChaptersFragmentDirections.viewSections(
67+
chapterId = chapter.id
68+
)
69+
)
70+
}
71+
val layoutManager = LinearLayoutManager(requireContext())
72+
binding.chapters.layoutManager = layoutManager
73+
val itemDivider = DividerItemDecoration(requireContext(), layoutManager.orientation)
74+
binding.chapters.addItemDecoration(itemDivider)
75+
binding.chapters.adapter = adapter
5776
}
77+
5878
}
5979
}
6080
}

‎app/src/main/java/guide/graphql/toc/SectionsAdapter.kt

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
package guide.graphql.toc
22

3+
import android.content.Context
34
import android.view.LayoutInflater
45
import android.view.ViewGroup
56
import androidx.recyclerview.widget.RecyclerView
67
import guide.graphql.toc.databinding.SectionBinding
78

89
class SectionsAdapter(
910
private val chapterNumber: Int?,
10-
private val sections: List<SectionsQuery.Section>
11+
private val sections: List<SectionsQuery.Section?>,
12+
private val context: Context
1113
) :
1214
RecyclerView.Adapter<SectionsAdapter.ViewHolder>() {
1315

@@ -24,7 +26,8 @@ class SectionsAdapter(
2426

2527
override fun onBindViewHolder(holder: ViewHolder, position: Int) {
2628
val section = sections[position]
27-
holder.binding.sectionTitle.text =
28-
if (section.number == null) section.title else "${chapterNumber}.${section.number}: ${section.title}"
29+
section?.let {
30+
holder.binding.sectionTitle.text = context.getString(R.string.section_title, chapterNumber.toString(), section.number.toString(), section.title)
31+
}
2932
}
3033
}

‎app/src/main/java/guide/graphql/toc/SectionsFragment.kt

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,16 @@ import android.os.Bundle
44
import android.view.LayoutInflater
55
import android.view.View
66
import android.view.ViewGroup
7+
import androidx.appcompat.app.AppCompatActivity
78
import androidx.fragment.app.Fragment
89
import androidx.lifecycle.lifecycleScope
10+
import androidx.navigation.fragment.findNavController
911
import androidx.navigation.fragment.navArgs
12+
import androidx.recyclerview.widget.DividerItemDecoration
1013
import androidx.recyclerview.widget.LinearLayoutManager
1114
import com.apollographql.apollo.coroutines.toDeferred
1215
import com.apollographql.apollo.exception.ApolloException
16+
import com.google.android.material.transition.MaterialSharedAxis
1317
import guide.graphql.toc.databinding.SectionsFragmentBinding
1418

1519
class SectionsFragment : Fragment() {
@@ -26,9 +30,24 @@ class SectionsFragment : Fragment() {
2630
return binding.root
2731
}
2832

33+
override fun onCreate(savedInstanceState: Bundle?) {
34+
super.onCreate(savedInstanceState)
35+
val forward = MaterialSharedAxis(MaterialSharedAxis.Z, true)
36+
enterTransition = forward
37+
38+
val backward = MaterialSharedAxis(MaterialSharedAxis.Z, false)
39+
returnTransition = backward
40+
}
41+
2942
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
3043
super.onViewCreated(view, savedInstanceState)
3144

45+
(requireActivity() as AppCompatActivity).setSupportActionBar(binding.chapterHeader)
46+
(requireActivity() as AppCompatActivity).supportActionBar?.setDisplayHomeAsUpEnabled(true)
47+
binding.chapterHeader.setNavigationOnClickListener {
48+
findNavController().navigateUp()
49+
}
50+
3251
lifecycleScope.launchWhenResumed {
3352
binding.spinner.visibility = View.VISIBLE
3453
binding.error.visibility = View.GONE
@@ -38,34 +57,43 @@ class SectionsFragment : Fragment() {
3857
SectionsQuery(id = args.chapterId)
3958
).toDeferred().await()
4059
} catch (e: ApolloException) {
41-
binding.spinner.visibility = View.GONE
42-
binding.error.text = "Error making the GraphQL request: ${e.message}"
43-
binding.error.visibility = View.VISIBLE
60+
showErrorMessage(getString(R.string.graphql_error, e.message))
4461
return@launchWhenResumed
4562
}
4663

4764
val chapter = response.data?.chapter
4865
if (chapter == null || response.hasErrors()) {
49-
binding.spinner.visibility = View.GONE
50-
binding.error.text = response.errors?.get(0)?.message
51-
binding.error.visibility = View.VISIBLE
66+
showErrorMessage(response.errors?.get(0)?.message ?: getString(R.string.error))
5267
return@launchWhenResumed
5368
}
5469

5570
val chapterNumber = chapter.number?.toInt()
5671
binding.spinner.visibility = View.GONE
5772
binding.chapterHeader.title =
58-
if (chapter.number == null) chapter.title else "Chapter ${chapterNumber}: ${chapter.title}"
73+
if (chapter.number == null) chapter.title else getString(
74+
R.string.chapter_title,
75+
chapter.number.toString(),
76+
chapter.title
77+
)
5978

6079
if (chapter.sections.size > 1) {
6180
val adapter =
62-
SectionsAdapter(chapterNumber, chapter.sections as List<SectionsQuery.Section>)
63-
binding.sections.layoutManager = LinearLayoutManager(requireContext())
81+
SectionsAdapter(chapterNumber, chapter.sections, requireContext())
82+
val layoutManager = LinearLayoutManager(requireContext())
83+
binding.sections.layoutManager = layoutManager
84+
val itemDivider = DividerItemDecoration(requireContext(), layoutManager.orientation)
85+
binding.sections.addItemDecoration(itemDivider)
6486
binding.sections.adapter = adapter
6587
} else {
66-
binding.error.text = "This chapter has no sections."
88+
binding.error.text = getString(R.string.no_section_error)
6789
binding.error.visibility = View.VISIBLE
6890
}
6991
}
7092
}
93+
94+
private fun showErrorMessage(error: String) {
95+
binding.spinner.visibility = View.GONE
96+
binding.error.text = error
97+
binding.error.visibility = View.VISIBLE
98+
}
7199
}

‎app/src/main/res/layout/activity_main.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
android:id="@+id/main_frame_layout"
88
tools:context=".MainActivity">
99

10-
<fragment
10+
<androidx.fragment.app.FragmentContainerView
1111
android:id="@+id/nav_host_fragment"
1212
android:name="androidx.navigation.fragment.NavHostFragment"
1313
android:layout_width="match_parent"

‎app/src/main/res/layout/chapter.xml

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,43 +5,33 @@
55
xmlns:tools="http://schemas.android.com/tools"
66
android:layout_width="match_parent"
77
android:layout_height="wrap_content"
8+
android:paddingStart="20dp"
9+
android:paddingTop="16dp"
10+
android:paddingBottom="16dp"
811
android:background="?selectableItemBackground">
912

1013
<TextView
1114
android:id="@+id/chapter_header"
1215
android:layout_width="0dp"
1316
android:layout_height="wrap_content"
14-
android:layout_marginStart="30dp"
15-
android:layout_marginTop="20dp"
16-
android:textSize="18sp"
17-
android:textStyle="bold"
17+
style="@style/AppTheme.Heading"
1818
app:layout_constraintBottom_toTopOf="@+id/chapter_subheader"
1919
app:layout_constraintEnd_toEndOf="parent"
2020
app:layout_constraintStart_toStartOf="parent"
2121
app:layout_constraintTop_toTopOf="parent"
22-
app:layout_constraintVertical_chainStyle="packed"></TextView>
22+
app:layout_constraintVertical_chainStyle="packed" />
2323

2424
<TextView
2525
android:id="@+id/chapter_subheader"
2626
android:layout_width="0dp"
27+
style="@style/AppTheme.Subheading"
2728
android:layout_height="wrap_content"
28-
android:layout_marginStart="30dp"
2929
android:layout_marginTop="10dp"
30-
android:layout_marginBottom="20dp"
31-
android:textSize="18sp"
3230
app:layout_constraintBottom_toBottomOf="parent"
3331
app:layout_constraintEnd_toEndOf="parent"
3432
app:layout_constraintRight_toRightOf="parent"
3533
app:layout_constraintStart_toStartOf="parent"
36-
app:layout_constraintTop_toBottomOf="@+id/chapter_header"></TextView>
34+
app:layout_constraintTop_toBottomOf="@+id/chapter_header" />
3735

38-
<View
39-
android:id="@+id/divider"
40-
android:layout_width="0dp"
41-
android:layout_height="1dp"
42-
android:background="@color/colorPrimaryDark"
43-
app:layout_constraintBottom_toBottomOf="parent"
44-
app:layout_constraintLeft_toLeftOf="parent"
45-
app:layout_constraintRight_toRightOf="parent" />
4636

4737
</androidx.constraintlayout.widget.ConstraintLayout>

‎app/src/main/res/layout/chapters_fragment.xml

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,24 @@
44
android:layout_height="match_parent"
55
xmlns:app="http://schemas.android.com/apk/res-auto">
66

7-
<Toolbar android:layout_width="0dp" android:layout_height="wrap_content"
8-
app:layout_constraintStart_toStartOf="parent"
9-
app:layout_constraintEnd_toEndOf="parent"
10-
app:layout_constraintTop_toTopOf="parent"
11-
android:background="@color/colorPrimary"
12-
android:title="@string/app_name"
13-
android:titleTextColor="@android:color/white"
14-
android:id="@+id/book_header"
15-
/>
7+
8+
9+
<com.google.android.material.appbar.AppBarLayout
10+
android:id="@+id/book_header_appbar"
11+
android:layout_width="match_parent"
12+
android:layout_height="wrap_content"
13+
android:background="@color/colorPrimary"
14+
app:layout_constraintEnd_toEndOf="parent"
15+
app:layout_constraintStart_toStartOf="parent"
16+
app:layout_constraintTop_toTopOf="parent">
17+
<androidx.appcompat.widget.Toolbar
18+
app:titleTextColor="?attr/colorOnPrimary"
19+
android:id="@+id/book_header"
20+
android:title="@string/app_name"
21+
android:layout_width="match_parent"
22+
android:layout_height="?attr/actionBarSize" />
23+
</com.google.android.material.appbar.AppBarLayout>
24+
1625

1726
<androidx.recyclerview.widget.RecyclerView
1827
android:id="@+id/chapters"
@@ -21,5 +30,5 @@
2130
app:layout_constraintBottom_toBottomOf="parent"
2231
app:layout_constraintEnd_toEndOf="parent"
2332
app:layout_constraintStart_toStartOf="parent"
24-
app:layout_constraintTop_toBottomOf="@+id/book_header" />
33+
app:layout_constraintTop_toBottomOf="@+id/book_header_appbar" />
2534
</androidx.constraintlayout.widget.ConstraintLayout>

0 commit comments

Comments
 (0)