Speakers issue #15 #48
Conversation
| private MaterialCardView cardView; | ||
|
|
||
|
|
||
| public static SpeakerCardFragment newInstance() { |
There was a problem hiding this comment.
This method is unnecessary as no arguments are passed
There was a problem hiding this comment.
Check line 105 of home activity, it is being used there.
There was a problem hiding this comment.
it can be replaced simply by new SpeakerCardFragment() (Also, you don't have to write an empty constructor i.e. public SpeakerCardFragment(){}, the compiler does that automatically for you by default)
|
|
||
| @Override | ||
| public void onClick(View view) { | ||
| Intent intentToSpeakersActivity = new Intent(view.getContext(), SpeakersActivity.class); |
There was a problem hiding this comment.
[just suggestion] action + type (= lauchSpeakersIntent) would've been better choice for var name
| @Override | ||
| public void onResume() { | ||
| super.onResume(); | ||
| cardView.setOnClickListener(this); |
There was a problem hiding this comment.
move this setOnClickListener to onCreate, and no need of cardView.setOnClickListener(null)
| * */ | ||
|
|
||
| public class SpeakersActivity extends FragmentActivity { | ||
| private ArrayList<String> nameList; |
There was a problem hiding this comment.
create model for speakers and use it
| imageV = itemView.findViewById(R.id.speaker_image); | ||
| nameTV = itemView.findViewById(R.id.speaker_name); | ||
| descTV = itemView.findViewById(R.id.speaker_desc); | ||
| } |
There was a problem hiding this comment.
create a public method populate(Speker), populate the vh in that method. Also, set onClickListener in vh
| @Override | ||
| public void onBindViewHolder(@NonNull final SpeakerViewHolder holder, final int position) { | ||
|
|
||
| holder.imageV.setImageURI(imgList.get(position)); |
There was a problem hiding this comment.
move to Adapter's populate method
| @Override | ||
| public void onClick(View view) { | ||
| String url = clickList.get(position); | ||
| Intent intent = new Intent(Intent.ACTION_VIEW); |
There was a problem hiding this comment.
| @@ -0,0 +1,17 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
| <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android" | |||
There was a problem hiding this comment.
this linearlayout is useless, remove it and keep only rv
| repositories { | ||
| google() | ||
| jcenter() | ||
| } | ||
| dependencies { | ||
| classpath 'com.android.tools.build:gradle:3.2.0-alpha18' | ||
| classpath 'com.android.tools.build:gradle:3.2.0-beta01' |
| tools:context=".speakers.SpeakersActivity"> | ||
|
|
||
| <androidx.recyclerview.widget.RecyclerView | ||
| android:id="@+id/speakers_RV" |
There was a problem hiding this comment.
adapter can be set here in xml itself!
| app:placeholderImage="@mipmap/ic_launcher_round" | ||
| app:roundAsCircle="true" /> | ||
|
|
||
| <androidx.cardview.widget.CardView |
There was a problem hiding this comment.
remove the card, keep only tv, same for description
| android:layout_marginTop="0dp" | ||
| android:layout_weight="3" | ||
| app:actualImageScaleType="fitCenter" | ||
| app:placeholderImage="@mipmap/ic_launcher_round" |
There was a problem hiding this comment.
keep another image as placeholder (maybe person icon)
| android:id="@+id/speaker_image" | ||
| android:layout_width="match_parent" | ||
| android:layout_height="wrap_content" | ||
| android:layout_marginTop="0dp" |
There was a problem hiding this comment.
0 margin is default, remove this
| android:layout_height="wrap_content" | ||
| android:layout_margin="1dp" | ||
|
|
||
| android:layout_weight="0.5" |
There was a problem hiding this comment.
remove the weight, same for desc
| android:layout_width="match_parent" | ||
| android:layout_height="wrap_content" | ||
| android:layout_gravity="center" | ||
| android:text="He is a cool guy." |
There was a problem hiding this comment.
replace by tools:text="He is a cool guy.", same for name
| app:actualImageScaleType="fitCenter" | ||
| app:placeholderImage="@mipmap/ic_launcher_round" | ||
| app:roundAsCircle="true" /> | ||
|
|
There was a problem hiding this comment.
TODO in next issue/pr, social links of the speaker here @tanmaydixit
|
@tanmaydixit rebase to latest code |
Issue #15 solved credits @Hariram-R
dependencies made latest too.