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

feat: (Core) Custom Messages in pickers #4361

Merged
merged 14 commits into from
Jan 26, 2021
Merged

Conversation

JKMarkowski
Copy link
Contributor

Please provide a link to the associated issue.

closes: #2934

Please provide a brief summary of this pull request.

There is added new way to add messages into date/time/datetimepicker.

  • Added message Input into them.

I created new popover-form service, that can be reused in other form components in future.

Please check whether the PR fulfills the following requirements

Documentation checklist:

@JKMarkowski JKMarkowski added the enhancement New feature or request label Jan 18, 2021
@JKMarkowski JKMarkowski added this to the Sprint 54 - Los Angeles milestone Jan 18, 2021
@JKMarkowski JKMarkowski requested a review from a team January 18, 2021 09:45
@JKMarkowski JKMarkowski added this to In progress in Development via automation Jan 18, 2021
@JKMarkowski JKMarkowski self-assigned this Jan 18, 2021
@JKMarkowski JKMarkowski changed the title feat: Custom Messages in pickers feat: (Core) Custom Messages in pickers Jan 18, 2021
@JKMarkowski JKMarkowski added the core Core library specific issues label Jan 18, 2021
@netlify
Copy link

netlify bot commented Jan 18, 2021

✔️ Deploy preview for fundamental-ngx ready!

🔨 Explore the source changes: 3662074

🔍 Inspect the deploy logs: https://app.netlify.com/sites/fundamental-ngx/deploys/600558a6b5edd60008f5a3d9

😎 Browse the preview: https://deploy-preview-4361--fundamental-ngx.netlify.app

@netlify
Copy link

netlify bot commented Jan 18, 2021

Deploy preview for fundamental-ngx ready!

Built with commit 26efdc3

https://deploy-preview-4361--fundamental-ngx.netlify.app

Copy link
Contributor

@InnaAtanasova InnaAtanasova left a comment

Choose a reason for hiding this comment

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

Awesome work. Added some minor comments

Comment on lines 170 to 176
get state(): FormStates {
return this._state;
}
private _state: FormStates = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

missing typedocs and @hidden for _state

@ViewChild(CalendarComponent)
calendarComponent: CalendarComponent<D>;

@ViewChild('inputGroupComponent', { read: ElementRef })
Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -218,6 +240,19 @@ export class DatePickerComponent<D> implements OnInit, OnDestroy, ControlValueAc
@Output()
readonly activeViewChange: EventEmitter<FdCalendarView> = new EventEmitter<FdCalendarView>();

/** @hidden */
@ViewChild(CalendarComponent)
calendarComponent: CalendarComponent<D>;
Copy link
Contributor

Choose a reason for hiding this comment

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

should be prefixed with _

Comment on lines 250 to 254
/** @hidden The value of the input */
inputFieldDate: string = null;

/** @hidden Whether the date input is invalid */
isInvalidDateInput = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be prefixed with _ since it's hidden?

Comment on lines 188 to 191
get state(): FormStates {
return this._state;
}

private _state: FormStates = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above, missing typedocs and hidden

@@ -247,6 +280,10 @@ export class DatetimePickerComponent<D> implements OnInit, OnDestroy, OnChanges,
@ViewChild(CalendarComponent)
_calendarComponent: CalendarComponent<D>;

/** @hidden */
@ViewChild('inputGroupComponent', { read: ElementRef })
inputGroupElement: ElementRef;
Copy link
Contributor

Choose a reason for hiding this comment

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

_ prefix

@@ -8,3 +8,7 @@
max-width: 100%;
}
}

.invisible {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should have some sort of prefix, o.w. can conflict with the same class in the application

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not used anywhere, so I will just remove

@JKMarkowski
Copy link
Contributor Author

Thanks @InnaAtanasova for review, your comments are applied

@JKMarkowski JKMarkowski merged commit 857eba6 into main Jan 26, 2021
Development automation moved this from In progress to Done Jan 26, 2021
@JKMarkowski JKMarkowski deleted the feat/2934-custom-messages branch January 26, 2021 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core library specific issues enhancement New feature or request
Projects
No open projects
Development
  
Done
Development

Successfully merging this pull request may close these issues.

Update the time picker examples with custom messages
3 participants