- 
                Notifications
    You must be signed in to change notification settings 
- Fork 1.8k
fix: resolve whack-a-mole behavior of time/date pickers & add validations to date-time #3383
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
Conversation
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.
#2077 Still exists @CosmicCoder96
| Also #2366 | 
| // break; | ||
| // case 'end': | ||
| // defaultOptions.startCalendar = this.$().closest('.fields').find('.ui.calendar.time.picker'); | ||
| // break; | 
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.
@CosmicCoder96 Remove the comments, Else tested every widget works well 🎉
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.
leaving these comments.. because someday we might want the time validation to change it's values according to its start counterpart, removed it for now because jquery ui calendar does not compare date while matching time fields, which sort of broke the UI.
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.
Tested components :
- Wizard step 1 (start and end dates)
- ticket sales dates
- session-speaker forms
They are working fine. Marking LGTM
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.
| 
 Not reproducible on my local, Any specific steps? | 
| @kushthedude just edit the time and date while creating the discount code. You'll see it | 
| @shreyanshdwivedi @kushthedude The behavior shreyansh noted might be for some discount codes who previously had a null value. I can't recreate it while creating new ones or existing valid ones. Though I did was able to recreate that when I forced a false value for the start time inside DB. | 
| 
 you mean edit using backspace? | 
| @CosmicCoder96 there was a weird behavior. When I followed links to create discount code and edited it, I was able to reproduce it. | 
| @shreyanshdwivedi Caught the source, thanks for the review 👍 fixed now. | 
| }); | ||
| let currentDiscountCode = model.discountCode; | ||
| let event = this.modelFor('events.view'); | ||
| currentDiscountCode.set('validFrom', moment().toISOString()); | 
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.
this was the problem 😈  converting it to string made it invalid to moment js computations.
@shreyanshdwivedi @kushthedude

Fixes #3020
Fixes #3382
Fixes #2077
Fixes #2366
Fixes #2972