-
Notifications
You must be signed in to change notification settings - Fork 551
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
feat: Added empty view for events fragment #1157
feat: Added empty view for events fragment #1157
Conversation
import kotlinx.android.synthetic.main.fragment_events.view.progressBar | ||
import kotlinx.android.synthetic.main.fragment_events.view.shimmerEvents | ||
import kotlinx.android.synthetic.main.fragment_events.view.swiperefresh | ||
import kotlinx.android.synthetic.main.fragment_events.view.* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid wildcard imports.
android:layout_height="450dp" | ||
android:text="@string/no_events_message" | ||
android:gravity="center" | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Newline not required.
<TextView | ||
android:id="@+id/noEventsMessage" | ||
android:layout_width="match_parent" | ||
android:layout_height="450dp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please define all dimensions in dimension resources file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will fix it now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed it, please review
import kotlinx.android.synthetic.main.fragment_events.view.progressBar | ||
import kotlinx.android.synthetic.main.fragment_events.view.shimmerEvents | ||
import kotlinx.android.synthetic.main.fragment_events.view.homeScreenLL | ||
import kotlinx.android.synthetic.main.fragment_events.view.locationTextView |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why these changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry for that, I will make sure I learn how to check for difference of files carefully before making a push and PR, I fixed that
…nhanh11001/open-event-android into 1141_empty_view_event_fragment
<TextView | ||
android:id="@+id/noEventsMessage" | ||
android:layout_width="match_parent" | ||
android:layout_height="@dimen/empty_view_height" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make it wrap content
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@iamareebjamal please review again, I've made my change, I was having problem to make the empty-event message in the center of screen, but then I figured out that the NestedScrollView doesn't fill up the screen when there are no enough data. So I put the fillViewPort="true" in.
@liveHarshit please review when you have the time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fixes #1141 Adding empty view for EventsFragment
Screenshots for the change: