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

docs: new docs for timepicker #708

Merged
merged 2 commits into from Apr 10, 2019
Merged

Conversation

mikerodonnell89
Copy link
Member

fixes #690

@@ -21,44 +21,64 @@ import { PopperOptions } from 'popper.js';
styles: [':host {display: inline-block;}']
})
export class TimePickerComponent implements ControlValueAccessor, OnInit {

/** @Input An object that contains three integer properties: 'hour' (ranging from 0 to 23),
* 'minute' (ranging from 0 to 59), and 'second' (ranging from 0 to 59). */
@Input()
time: TimeObject = { hour: 0, minute: 0, second: 0 };
Copy link
Contributor

Choose a reason for hiding this comment

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

As mentioned in the other PR, consider using markdown to make this look a bit nicer, maybe a list or something?

also maybe explain what it is used for on top of what it consists of

@Input()
time: TimeObject = { hour: 0, minute: 0, second: 0 };

/** @Input Uses compact time picker. */
Copy link
Contributor

Choose a reason for hiding this comment

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

Whether the time picker should be in compact mode. maybe?
Uses compact time picker isnt very clear

@Input()
compact: boolean = false;

/** @Input Boolean, when set to true, uses the 24 hour clock (hours ranging from 0 to 23)
* and does not display a period control. Default is false. */
@Input()
meridian: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

remove 'Boolean,' and also 'Default is false', consider adding boolean = false to the input

@Input()
disabled: boolean;

/** @Input Boolean, when set to false, hides the buttons that increment and decrement the corresponding input. Default is true. */
@Input()
spinners: boolean = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

remove 'Boolean,' and 'Default is..'

@Input()
spinners: boolean = true;

/** @Input Boolean, when set to false, hides the input for seconds. Default is true. */
@Input()
displaySeconds: boolean = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

remove 'Boolean,' and 'Default is..'

ngOnInit(): void {
this.placeholder = this.getPlaceholder();
}

/**
* @returns Returns the current value of the time input.
*/
getTime() {
Copy link
Contributor

Choose a reason for hiding this comment

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

If this returns something, the return type should be explicitly given by the function.

In that case, you would not need the @returns, typedoc will infer the return type. You'd just need a small explanation of what the method does.

@@ -108,6 +128,7 @@ export class TimePickerComponent implements ControlValueAccessor, OnInit {
return formattedTime !== undefined ? formattedTime : '';
Copy link
Contributor

Choose a reason for hiding this comment

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

should also identify the return type in the method head (not part of the docs issue tho)

@@ -215,29 +241,35 @@ export class TimePickerComponent implements ControlValueAccessor, OnInit {
return retVal;
Copy link
Contributor

Choose a reason for hiding this comment

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

return type

@MattL75 MattL75 changed the title chore: new docs for timepicker docs: new docs for timepicker Apr 9, 2019
@mikerodonnell89 mikerodonnell89 merged commit 4a17c73 into master Apr 10, 2019
@MattL75 MattL75 deleted the chore/#690-timepicker-docs branch April 12, 2019 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document Timepicker component
2 participants