Skip to content
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

Closed
wants to merge 1 commit into from

Conversation

haran2248
Copy link
Contributor

@haran2248 haran2248 commented Feb 9, 2021

Issue Fix

Fixes #1216

Screenshots

WhatsApp Image 2021-02-10 at 01 23 36

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 anything

  • If you have multiple commits please combine them into one commit by squashing them.

@haran2248
Copy link
Contributor Author

haran2248 commented Feb 9, 2021

@devansh-299 Please review this

@haran2248 haran2248 changed the title fix#1216:Minimum Date in valid Till DatePicker fixed. fix#1216:Minimum Date in *Valid Till* DatePicker fixed. Feb 10, 2021
@@ -58,6 +65,8 @@ class SIDetailsActivity : BaseActivity(), StandingInstructionContract.SIDetailsV
setUpUI()
}

@SuppressLint("SimpleDateFormat")
@RequiresApi(Build.VERSION_CODES.O)
Copy link
Collaborator

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?

Copy link
Contributor Author

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

Copy link
Contributor Author

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

Copy link
Collaborator

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?

Copy link
Contributor Author

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
Copy link
Collaborator

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

Copy link
Contributor Author

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

@haran2248
Copy link
Contributor Author

@devansh-299 please review this

@@ -41,6 +47,7 @@ class SIDetailsActivity : BaseActivity(), StandingInstructionContract.SIDetailsV
private lateinit var mStandingInstructionPresenter:
StandingInstructionContract.StandingInstructorDetailsPresenter

@SuppressLint("NewApi")
Copy link
Contributor

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?

Copy link
Contributor Author

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

Comment on lines 3 to 8
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
Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@alphaNewrex
Copy link
Contributor

@devansh-299 can I work on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Valid till dates can be edited to made lass than Valid from
5 participants