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
chore: move date-picker out into a library #1172
chore: move date-picker out into a library #1172
Conversation
DEV-2502 Move date-picker out into library
We need it in the advanced search so we will need to move it from the app into a library |
@@ -1,12 +1,10 @@ | |||
import { Component, Inject, Input, OnDestroy, OnInit } from '@angular/core'; | |||
import { | |||
UntypedFormBuilder, | |||
import {UntypedFormBuilder, |
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.
What is here the code convention for multiple imports?
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.
Basically whatever the formatter chooses when you right click > format document
if (calendar === 'GREGORIAN') { | ||
const calDate = new GregorianCalendarDate( | ||
new CalendarPeriod(date, date) | ||
); | ||
return calDate.daysInMonth(date); | ||
} else if (calendar === 'JULIAN') { | ||
const calDate = new JulianCalendarDate( | ||
new CalendarPeriod(date, date) | ||
); | ||
return calDate.daysInMonth(date); | ||
} else if (calendar === 'ISLAMIC') { | ||
const calDate = new IslamicCalendarDate( | ||
new CalendarPeriod(date, date) | ||
); | ||
return calDate.daysInMonth(date); | ||
} else { | ||
throw Error('Unknown calendar ' + calendar); | ||
} |
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 mandatory but in case there are more than two if/ else if lines you can use switch case
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 agree but I didn't want to change too much of the code. I'd rather refactor it in a separate PR.
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
resolves DEV-2502