Skip to content

IgxTimePicker refactoring#8947

Merged
Lipata merged 53 commits intopickers-refactoringfrom
PMiteva/time-picker-refactoring
Apr 19, 2021
Merged

IgxTimePicker refactoring#8947
Lipata merged 53 commits intopickers-refactoringfrom
PMiteva/time-picker-refactoring

Conversation

@PlamenaMiteva
Copy link
Contributor

@PlamenaMiteva PlamenaMiteva commented Feb 10, 2021 •
edited by Lipata
Loading

Closes #6482

Additional information (check all that apply):

  • Bug fix
  • New functionality
  • Documentation
  • Demos
  • CI/CD

Checklist:

  • All relevant tags have been applied to this PR
  • This PR includes unit tests covering all the new code (test guidelines)
  • This PR includes API docs for newly added methods/properties (api docs guidelines)
  • This PR includes feature/README.MD updates for the feature docs
  • This PR includes general feature table updates in the root README.MD
  • This PR includes CHANGELOG.MD updates for newly added functionality
  • This PR contains breaking changes
  • This PR includes ng update migrations for the breaking changes (migrations guidelines)
  • This PR includes behavioral changes and the feature specification has been updated with them

@PlamenaMiteva
refactor(time-picker): initial refactoring #6482
e309406
@PlamenaMiteva
Merge branch 'pickers-refactoring' of https://github.com/IgniteUI/ign…
0dd4ae4 
…iteui-angular into PMiteva/time-picker-refactoring

# Conflicts:
#	projects/igniteui-angular/src/lib/time-picker/time-picker.component.ts
jackofdiamond5
jackofdiamond5 requested changes Feb 23, 2021
View reviewed changes
@jackofdiamond5 jackofdiamond5 mentioned this pull request Mar 12, 2021
Merged
14 tasks
@PlamenaMiteva
Merge branch 'pickers-refactoring' of https://github.com/IgniteUI/ign…
f116c9b 
…iteui-angular into PMiteva/time-picker-refactoring

# Conflicts:
#	projects/igniteui-angular/src/lib/date-common/pickers-base.directive.ts
@PlamenaMiteva
refactor(time-picker): add time pipe #6482
c4d528f
@PlamenaMiteva
refactor(time-picker): container refactoring #6482
f28fda0
@PlamenaMiteva
Merge branch 'pickers-refactoring' of https://github.com/IgniteUI/ign…
be97844 
…iteui-angular into PMiteva/time-picker-refactoring

# Conflicts:
#	projects/igniteui-angular/src/lib/date-common/picker-base.directive.ts
#	projects/igniteui-angular/src/lib/date-picker/date-picker.utils.ts
#	projects/igniteui-angular/src/lib/time-picker/time-picker.common.ts
#	projects/igniteui-angular/src/lib/time-picker/time-picker.component.spec.ts
#	projects/igniteui-angular/src/lib/time-picker/time-picker.component.ts
#	projects/igniteui-angular/src/lib/time-picker/time-picker.directives.ts
#	src/app/time-picker/time-picker.sample.ts
@PlamenaMiteva
Merge branch 'PMiteva/time-picker-refactoring' of https://github.com/…
eab163d 
…IgniteUI/igniteui-angular into PMiteva/time-picker-refactoring

# Conflicts:
#	projects/igniteui-angular/src/lib/date-common/picker-base.directive.ts
@PlamenaMiteva
refactor(time-picker): incorporate ISO format implementation #6482
e155b2e
@PlamenaMiteva
refactor(time-picker): add input/display format validation #6482
384818f
@PlamenaMiteva
refactor(time-picker): add input/display format validation #6482
0a0e710
damyanpetev
damyanpetev reviewed Apr 15, 2021
View reviewed changes
projects/igniteui-angular/src/lib/date-common/util/date-time.util.ts Outdated
const format = timePart.replace(/\d/g, '0');
timePart = new MaskParsingService().replaceInMask(timePart, value,
{ format, promptChar: '' }, 0, value.length).value;
timePart = timePart.substr(0, timePart.length - 1);
Copy link
Member

@damyanpetev damyanpetev Apr 15, 2021

Choose a reason for hiding this comment

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

Yeah, taking everything except the last char won't work in too many scenarios - time portion has zone different than UTC (+00) or no zone at all in which case you'd be removing from the milliseconds most likely. What's more, I don't see the need to remove parts of the value or go as far as the mask parsing for this - you already have a date-only part, perhaps just combining it with the literal and the time portion will produce a valid date string?

Copy link
Member

@damyanpetev damyanpetev Apr 15, 2021

Choose a reason for hiding this comment

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

Also, might want to add some invalid date checks to this parse method, unless the consuming locations are supposed to check for that. The return type is described as Date | null but I don't see how the latter is possible in the current code

@Lipata Lipata added the squash-merge Merge PR with "Squash and Merge" option label Apr 15, 2021
@PlamenaMiteva
refactor(time-picker): addressing comments #6482
ef058be
@PlamenaMiteva
refactor(time-picker): fix lint errors #6482
c82f73c
@PlamenaMiteva
refactor(time-picker): fix failing tests #6482
fad482d
@PlamenaMiteva
refactor(time-picker): add aria attributes to selected items #6482
ae279d8
@PlamenaMiteva
refactor(time-picker): revert tabs component #6482
7dc7fbd
@PlamenaMiteva
Merge branch 'pickers-refactoring' of https://github.com/IgniteUI/ign…
fbdaa1a 
…iteui-angular into PMiteva/time-picker-refactoring
@PlamenaMiteva
refactor(time-picker): remove fontSet #6482
304a0a3
@PlamenaMiteva
Merge branch 'pickers-refactoring' of https://github.com/IgniteUI/ign…
ac685cb 
…iteui-angular into PMiteva/time-picker-refactoring
@PlamenaMiteva
refactor(time-picker): add readOnly property #6482
8e24a34
@PlamenaMiteva
refactor(time-picker): fix time picker settings in grid templates #6482
7185716
@PlamenaMiteva
refactor(time-picker): fix typos #6482
f19b98b
damyanpetev
damyanpetev reviewed Apr 19, 2021
View reviewed changes
projects/igniteui-angular/src/lib/time-picker/time-picker.component.html
[attr.role]="timeItem.isSelectedTime ? 'spinbutton' : null"
[attr.aria-valuenow]="timeItem.isSelectedTime ? timeItem.hourValue : null"
[attr.aria-valuemin]="timeItem.isSelectedTime ? timeItem.minValue : null"
[attr.aria-valuemax]="timeItem.isSelectedTime ? timeItem.maxValue : null"
Copy link
Member

@damyanpetev damyanpetev Apr 19, 2021

Choose a reason for hiding this comment

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

Since the time item is apparently aware if it's selected, these can all be host bound in the directive instead to reduce the amount of repetition in this template

projects/igniteui-angular/src/lib/time-picker/time-picker.component.ts
*/
@Input()
public disabled = false;
public displayFormat: string;
Copy link
Member

@damyanpetev damyanpetev Apr 19, 2021

Choose a reason for hiding this comment

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

Technically, that's inherited already, though just for the documentation it might be okay to keep. Same with a few others.

@Lipata Lipata merged commit b2b1758 into pickers-refactoring Apr 19, 2021
@Lipata Lipata deleted the PMiteva/time-picker-refactoring branch April 19, 2021 14:08
Lipata pushed a commit that referenced this pull request Apr 21, 2021
@jackofdiamond5 @PlamenaMiteva
refactor(date-picker, time-picker): date-time-editor integration #6482 …
403acdc 
…#6483 (#9160)

* feat(*): add PickersBaseDirective

* feat(date-time-editor): handling for wheel events

* feat(types): add new enum HeaderOrientation

* feat(date-time-editor): add preventSpeenOnWheel input

* feat(date-time-editor): can be set to suppress focus

* feat(DatePickerUtils): add method for min/max validation

* refactor(CalendarContainer): move to date-utils

* feat(PickersBaseDirective): implements EditorProvider

* refactor(picker-icons): move picker icon components to date-common

* feat(date-time-editor): add option to set spin delta per part #7169 (#8987)

* refactor(date-range): pickers base, overlay service, template buttons, valueChange

* refactor(date-editor): rename isSpinLoop to spinLoop

* feat(date-time-editor, date-range-picker, date-picker): ISO 8601 support #6994

* feat(date-picker): add readonly prop, close on escape

* refactor(advanced-filtering): update date picker template

* refactor(filtering-row): update date picker template

* refactor(excel-style-date-expr): update date picker template

* refactor(input-directive):  set disabled using hostbinding

* feat(igx-time-picker): refactoring #6482 (#8947)

* feat(date-time-editor, pickers): add migrations, changelog, readme #6482, #6483 (#9319)

Co-authored-by: Boris <jackofdiamond596@gmail.com>
Co-authored-by: plamenamiteva <plamenamiteva@abv.bg>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@damyanpetev damyanpetev damyanpetev approved these changes

@jackofdiamond5 jackofdiamond5 jackofdiamond5 approved these changes

Assignees

No one assigned

Labels

🕐 time-picker refactoring squash-merge Merge PR with "Squash and Merge" option

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@PlamenaMiteva @damyanpetev @jackofdiamond5 @Lipata