Issue #14 #21
Issue #14 #21
Conversation
| @@ -47,6 +47,9 @@ dependencies { | |||
| testImplementation(TestLibraries.jUnit) | |||
| androidTestImplementation(TestLibraries.runnner) | |||
| androidTestImplementation(TestLibraries.espressoCore) | |||
|
|
|||
| //Third Party Libraries | |||
There was a problem hiding this comment.
lets keep it. there are other comments like this in this file.
There was a problem hiding this comment.
I think we can delete all comments in the file. naming is clear enough, we don't need to comments.
| var eventID = args.eventID | ||
|
|
||
| viewModel.getEvent(eventID) | ||
|
|
There was a problem hiding this comment.
We could delete unnecessary spaces.
| @@ -0,0 +1,166 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
| <ScrollView xmlns:android="http://schemas.android.com/apk/res/android" | |||
There was a problem hiding this comment.
all dp values can coming from dimens folder
| android:layout_width="match_parent" | ||
| android:layout_height="match_parent"> | ||
|
|
||
| <androidx.constraintlayout.widget.ConstraintLayout |
There was a problem hiding this comment.
Using ConstraintLayout in ConstraintLayout isn't recommended situation. Is there any better way?
There was a problem hiding this comment.
If ConstrainsLayout is used somewhere, we shouldn't need another layout type. That's because you can position any view wherever you want. ConstraintLayout provides this functionality with its powerful attributes.
| @@ -28,5 +29,8 @@ class EventListFragment : Fragment() { | |||
| val eventID = (0..99999999).random() | |||
| navigate(EventListFragmentDirections.actionEventListFragmentToUserListFragment(eventID)) | |||
| } | |||
|
|
|||
| // val event:Event = Event() | |||
There was a problem hiding this comment.
We could delete this if It's unnecessary
|
|
||
| @Serialization |
There was a problem hiding this comment.
I think you don't need to use it. Moshi provides that
| //TODO need event photo link | ||
| // Glide.with(context) | ||
| // .load(event.) | ||
| // .transition(DrawableTransitionOptions.withCrossFade()) | ||
| // .skipMemoryCache(true) | ||
| // .into(ivImage) |
There was a problem hiding this comment.
hmm I checked the api and seems like we are not able to get the image from the api so we need to remove the image and move the rest of the page up
| btnCheckIn.setOnClickListener { | ||
| //TODO attendees page | ||
| } |
There was a problem hiding this comment.
please do. navigate to UserListFragment and use safe args to pass eventid.
| <com.google.android.material.textview.MaterialTextView | ||
| android:id="@+id/txtEventName" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_gravity="center_horizontal" | ||
| android:layout_marginTop="24dp" | ||
| android:fontFamily="@font/nunito_extrabold" | ||
| android:textColor="@color/dark_grey_blue" | ||
| android:textSize="16sp" | ||
| app:layout_constraintEnd_toEndOf="parent" | ||
| app:layout_constraintStart_toStartOf="parent" | ||
| app:layout_constraintTop_toTopOf="parent" | ||
| tools:text="GDG DevFest Istanbul 2019" /> | ||
|
|
||
| <com.google.android.material.textview.MaterialTextView | ||
| android:id="@+id/txtEventDate" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_marginStart="16dp" | ||
| android:layout_marginTop="16dp" | ||
| android:fontFamily="@font/nunito_semibold" | ||
| android:textColor="@color/text_color" | ||
| android:textSize="12sp" | ||
| app:layout_constraintStart_toStartOf="parent" | ||
| app:layout_constraintTop_toBottomOf="@id/txtEventName" | ||
| tools:text="24 Aug, 2pm" /> | ||
|
|
||
| <com.google.android.material.textview.MaterialTextView | ||
| android:id="@+id/txtEventVenue" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_marginTop="4dp" | ||
| android:fontFamily="@font/nunito_semibold" | ||
| android:textColor="@color/text_color" | ||
| android:textSize="12sp" | ||
| app:layout_constraintStart_toStartOf="@id/txtEventDate" | ||
| app:layout_constraintTop_toBottomOf="@id/txtEventDate" | ||
| tools:text="Kolektif House Maslak" /> |
There was a problem hiding this comment.
Use or add text styles defined in app/src/main/res/values/type.xml
| @@ -47,6 +47,9 @@ dependencies { | |||
| testImplementation(TestLibraries.jUnit) | |||
| androidTestImplementation(TestLibraries.runnner) | |||
| androidTestImplementation(TestLibraries.espressoCore) | |||
|
|
|||
| //Third Party Libraries | |||
There was a problem hiding this comment.
lets keep it. there are other comments like this in this file.
| txtEventDate.text = event.localDate | ||
| txtEventVenue.text = event.venue?.name | ||
|
|
||
| txtAttendee.text = "${event.yesRsvpCount} Attendees" |
There was a problem hiding this comment.
Please use strings.xml
| const val moshiConverter = "com.squareup.retrofit2:converter-moshi:2.5.0" | ||
| const val glide = "com.github.bumptech.glide:glide:4.10.0" |
There was a problem hiding this comment.
Versions should be defined in the separated class.
No description provided.