-
Notifications
You must be signed in to change notification settings - Fork 441
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
fix#1216:Minimum Date in *Valid Till* DatePicker fixed. #1217
Conversation
@devansh-299 Please review this |
@@ -58,6 +65,8 @@ class SIDetailsActivity : BaseActivity(), StandingInstructionContract.SIDetailsV | |||
setUpUI() | |||
} | |||
|
|||
@SuppressLint("SimpleDateFormat") | |||
@RequiresApi(Build.VERSION_CODES.O) |
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.
@haran2248 setting the min date should work on all the devices supported, hence requiring API version26+ for this is probably not the best way. Can you figure out any other way of doing this?
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.
yeah ill work on it
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 requires api part was unnecessary i guess
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 think it's needed. See there's a need to suppress the warning for using a feature not supported (or introduced later) by the specified min SDK version. Did you explore other methods?
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.
Yeah I just found a simple fix to that warning by adding locale.getDefault(),Now the warrning has disappeared,and it works fine without any additional support
@@ -68,6 +77,7 @@ class SIDetailsActivity : BaseActivity(), StandingInstructionContract.SIDetailsV | |||
DatePickerDialog.OnDateSetListener { view, year, monthOfYear, dayOfMonth -> | |||
tv_valid_till.text = "$dayOfMonth-${(monthOfYear + 1)}-$year" | |||
}, year, month, day) | |||
picker.datePicker.minDate=SimpleDateFormat("dd-MM-yyyy").parse(tv_valid_from.text.toString()).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.
Also always have a white space before and after the =
. The Kotlin checks are currently not in place, so till the time they are, please try to properly format the code from your end only.
We are following this for our Java code. This can also be followed for .kt
files. Do check it out
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.
oh my bad,ill correct it
4c365ac
to
47907ad
Compare
@devansh-299 please review this |
@@ -41,6 +47,7 @@ class SIDetailsActivity : BaseActivity(), StandingInstructionContract.SIDetailsV | |||
private lateinit var mStandingInstructionPresenter: | |||
StandingInstructionContract.StandingInstructorDetailsPresenter | |||
|
|||
@SuppressLint("NewApi") |
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.
Any justification for this suppressing lint for this?
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.
Yeah i have removed that right now,That was in accordance to the previous commit
06047ea
to
eeab9ad
Compare
import android.annotation.SuppressLint | ||
import android.app.DatePickerDialog | ||
import android.content.res.Resources | ||
import android.os.Build | ||
import android.os.Bundle | ||
import android.support.annotation.RequiresApi |
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.
Remove these redundant imports, Ctrl+Alt+O
kotlin files before committing.
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.
done
d6ebf10
to
25ce010
Compare
d9ba446
to
9bc7822
Compare
@devansh-299 can I work on this? |
Issue Fix
Fixes #1216
Screenshots
Description
The valid Till DatePicker Dialogues has been fixed so that the valid till date is chronologically never less than valid from.
Few additional warnings have also been rectified along the way
Apply the
AndroidStyle.xml
style template to your code in Android Studio.Run the unit tests with
./gradlew check
to make sure you didn't break anythingIf you have multiple commits please combine them into one commit by squashing them.