Skip to content

feat(datepicker): add timepicker support#6275

Merged
valorkin merged 12 commits intovalor-software:developmentfrom
ErikYu:feat-add-datetimepicker
Sep 24, 2021
Merged

feat(datepicker): add timepicker support#6275
valorkin merged 12 commits intovalor-software:developmentfrom
ErikYu:feat-add-datetimepicker

Conversation

@ErikYu
Copy link
Copy Markdown
Contributor

@ErikYu ErikYu commented Aug 30, 2021

Fixes #551

PR Checklist

Before creating new PR, please take a look at checklist below to make sure that you've done everything that needs to be done before we can merge it.

  • read and followed the CONTRIBUTING.md guide.
  • built and tested the changes locally.
  • added/updated tests.
  • added/updated API documentation.
  • added/updated demos.

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 30, 2021

Codecov Report

Merging #6275 (a4d3616) into development (3a0757a) will decrease coverage by 0.02%.
The diff coverage is 73.61%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #6275      +/-   ##
===============================================
- Coverage        77.06%   77.04%   -0.03%     
===============================================
  Files              311      312       +1     
  Lines            10793    10863      +70     
  Branches          2599     2612      +13     
===============================================
+ Hits              8318     8369      +51     
- Misses            2463     2482      +19     
  Partials            12       12              
Impacted Files Coverage Δ
apps/ngx-bootstrap-docs/src/ng-api-doc.ts 100.00% <ø> (ø)
src/datepicker/reducer/bs-datepicker.state.ts 71.42% <ø> (ø)
src/datepicker/themes/bs/bs-datepicker-view.html 100.00% <ø> (ø)
src/datepicker/bs-datepicker-inline.component.ts 58.02% <25.00%> (-1.72%) ⬇️
src/datepicker/reducer/bs-datepicker.actions.ts 66.66% <33.33%> (-1.67%) ⬇️
src/datepicker/base/bs-datepicker-container.ts 31.11% <50.00%> (+0.87%) ⬆️
...ker/themes/bs/bs-datepicker-container.component.ts 75.00% <64.28%> (-2.34%) ⬇️
...hemes/bs/bs-daterangepicker-container.component.ts 64.56% <72.22%> (+0.93%) ⬆️
src/datepicker/reducer/bs-datepicker.reducer.ts 69.86% <82.35%> (+1.05%) ⬆️
src/datepicker/bs-datepicker.component.ts 75.93% <100.00%> (+0.74%) ⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a0757a...a4d3616. Read the comment docs.

@cypress
Copy link
Copy Markdown

cypress Bot commented Aug 30, 2021



Test summary

814 16 14 0


Run details

Project ngx-bootstrap
Status Failed
Commit 2c88567 ℹ️
Started Sep 21, 2021 8:48 AM
Ended Sep 21, 2021 9:00 AM
Duration 12:22 💡
OS Linux Ubuntu - 20.04
Browser Electron 87

View run in Cypress Dashboard ➡️


Failures

modals_service_page_spec.ts Failed
1 Modals demo page testing suite: Service examples > Component modals with interceptor > when user clicks on "Close" button, triggers the interceptor and doesn't close the modal
2 Modals demo page testing suite: Service examples > Component modals with interceptor > when user clicks on "Yes" button, closes the modal
3 Modals demo page testing suite: Service examples > Component modals with interceptor > when user clicks on "No" button, doesn't close the modal
carousel_page_spec.ts Failed
1 Carousel page testing suite > Slide changed event > example contains slides, indicators, left and right controls and "Slide has been switched: 0"
2 Carousel page testing suite > Slide changed event > when user click on left arrow - info changed to "Slide has been switched: 2"
3 Carousel page testing suite > Slide changed event > when user click on left arrow again - info changed to "Slide has been switched: 1"
4 Carousel page testing suite > Slide changed event > when user click on right arrow - info changed to "Slide has been switched: 1"
5 Carousel page testing suite > Slide changed event > when user click on right arrow again - info changed to "Slide has been switched: 2"
pagination_page_spec.ts Failed
1 Pagination demo page testing suite > Pager > example contains: 7 pages, Next - active, Previous - active, 4th page active
sortable_page_spec.ts Failed
1 Sortable demo page testing suite > Custom item template > example contains 2 bs-sortable (the 1st contain 4 items, 2d - 2) by default, each item in the first bs-sortable have index (starting from 0), under each component user see code-preview with appropriate models
This comment includes only the first 10 test failures. See all 16 failures in the Cypress Dashboard.

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

daniloff200
daniloff200 previously approved these changes Aug 31, 2021
@valorkin valorkin added this to the 2021-09-10 milestone Sep 10, 2021
return this._daysCalendar$;
}

selectedTime!: Observable<Date[]|undefined>;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

make it with optional type
enforce non nullable is no go

Comment thread src/datepicker/bs-datepicker.module.ts Outdated

@NgModule({
imports: [CommonModule, TooltipModule],
imports: [CommonModule, TooltipModule, FormsModule, TimepickerModule.forRoot()],
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

injecting forms here seem is a bit too much, you can do it without it
plus you should not use .forRoot() here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

.forRoot() is removed. And i added TimepickerActions in providers. Or there will be errors

view: state.view
};

const _time = (state.selectedTime || [])[0];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

move this code to a helper function copyTime
const _time = (state.selectedTime || [])[0]; this is a hack around
property access is faster and memory cheaper than creating empty array each time for no good reason

    if (state.selectedTime && state.selectedTime[0]) {
        const _time = state.selectedTime[0];
        newState.selectedDate.setHours(_time.getHours());
        newState.selectedDate.setMinutes(_time.getMinutes());
        newState.selectedDate.setSeconds(_time.getSeconds());
        newState.selectedDate.setMilliseconds(_time.getMilliseconds());
      }

Copy link
Copy Markdown
Contributor Author

@ErikYu ErikYu Sep 13, 2021

Choose a reason for hiding this comment

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

added one util function


case BsDatepickerActions.SELECT_TIME: {
const {date, index} = action.payload;
const selectedTime = [...state.selectedTime || []] ;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do proper checks and not || [] hacks please
you will get selectedTime is not iterable if it will be undefined

};

newState.selectedRange.forEach((dte: Date, index: number) => {
const _time = (state.selectedTime || [])[index];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

see above about copyTime

Comment thread src/datepicker/themes/bs/bs-datepicker-view.html Outdated
@valorkin valorkin modified the milestones: 2021-09-10, 2021-09-17 Sep 10, 2021
@valorkin valorkin temporarily deployed to ngx-bootstra-feat-add-d-q1qgpd September 24, 2021 13:16 Inactive
@valorkin valorkin merged commit e93950b into valor-software:development Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(datepicker): add DateTimePicker

3 participants