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
Switch location #82
Switch location #82
Conversation
Signed-off-by: Jeevan Surendran <jvns67@gmail.com>
Signed-off-by: Jeevan Surendran <jvns67@gmail.com>
Signed-off-by: Jeevan Surendran <jvns67@gmail.com>
Signed-off-by: Jeevan Surendran <jvns67@gmail.com>
app/src/main/java/com/checkin/app/checkin/home/fragments/UserLocationFragment.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/checkin/app/checkin/home/viewmodels/UserLocationViewModel.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/checkin/app/checkin/home/viewmodels/UserLocationViewModel.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/checkin/app/checkin/menu/viewmodels/BaseMenuViewModel.kt
Outdated
Show resolved
Hide resolved
liveViewModel.clearCartData.observe(this, Observer { }) | ||
|
||
mViewModel.setCityId(getCityId()) |
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.
You're calling fetchNearbyRestaurants twice because of setCityId call. No need to waste mobile data twice in getting the same data.
app/src/main/java/com/checkin/app/checkin/data/network/WebApiService.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/checkin/app/checkin/Shop/ShopRepository.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/checkin/app/checkin/home/epoxy/CityLocationModelHolder.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/checkin/app/checkin/home/epoxy/CurrentLocationModelHolder.kt
Outdated
Show resolved
Hide resolved
cityListener { _ -> | ||
val preferences = requireActivity().getSharedPreferences(Constants.LOCATION_CITY_FILE, Context.MODE_PRIVATE) | ||
with(preferences.edit()) { | ||
putInt(Constants.LOCATION_CITY_ID, model.id) |
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.
The coding pattern is really wrong here.
You're using a model variable bound to the one in loop. It's better to use the interface I mentioned above and declare the parameters for city data
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.
You still didn't change that. I already mentioned to properly use interface and use the parameters in the interface
Current location shouldn't be shown when typed |
04b8d64
to
60793c3
Compare
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.
There's still stuff to clean up.
cityListener { _ -> | ||
val preferences = requireActivity().getSharedPreferences(Constants.LOCATION_CITY_FILE, Context.MODE_PRIVATE) | ||
with(preferences.edit()) { | ||
putInt(Constants.LOCATION_CITY_ID, model.id) |
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.
You still didn't change that. I already mentioned to properly use interface and use the parameters in the interface
app/src/main/java/com/checkin/app/checkin/menu/listeners/LocationSelectedListener.java
Outdated
Show resolved
Hide resolved
app/src/main/java/com/checkin/app/checkin/menu/listeners/LocationSelectedListener.java
Outdated
Show resolved
Hide resolved
app/src/main/java/com/checkin/app/checkin/home/epoxy/CityLocationModelHolder.kt
Outdated
Show resolved
Hide resolved
val homeViewModel: HomeViewModel by activityViewModels() | ||
@BindView(R.id.et_user_location) | ||
internal lateinit var etUserLocation: EditText | ||
|
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.
Don't waste lines in between View fields... Fix that in all the files (like holder)
|
||
class UserLocationFragment : BaseFragment() { | ||
override val rootLayout = R.layout.fragment_user_location_switch | ||
val viewModel: UserLocationViewModel by viewModels() |
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.
Declare these variables after all the UI views are declared. Always keep rootLayout at the top, though.
app/src/main/java/com/checkin/app/checkin/home/epoxy/CityLocationModelHolder.kt
Outdated
Show resolved
Hide resolved
|
||
@BindView(R.id.tv_user_location_state) | ||
internal lateinit var tvStateLocation: TextView | ||
|
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.
Lots of needless blank lines here too. Always try and keep your code compact.
app/src/main/java/com/checkin/app/checkin/home/fragments/UserLocationFragment.kt
Outdated
Show resolved
Hide resolved
Signed-off-by: Jeevan Surendran <jvns67@gmail.com>
- Added swipe refresh in closed sessions listing - Cleaned Util logic to format duration - Fixed bug in UserLocationFragment not attached when selecting any option - Used default SharedPreferences for location too - Used proper way to use appbar by mentioning theme in manifest - Used the core manifest for new screens Signed-off-by: Shivansh Saini <shivanshs9@gmail.com>
Everything other than the main page where the location needs to be shown is implemented i guess.