-
Notifications
You must be signed in to change notification settings - Fork 1
Add Clock In #8
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
Add Clock In #8
Conversation
src/app/components/shared/details-fields/details-fields.component.spec.ts
Show resolved
Hide resolved
f0e1d01
to
ee5a5e6
Compare
src/app/components/options-sidebar/time-clock/time-clock.component.spec.ts
Show resolved
Hide resolved
it("should set showfileds as true", () => { | ||
const show = true; | ||
component.showFields = show; | ||
component.getShowFields(show); |
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.
It seems this function is changing showFields value. In this case, the field name should be:
setShowFields(show)
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.
Done 👌
|
||
getShowFields(show: boolean) { |
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.
definitely, the function name should be: setShowFields
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.
Done 👌
component.showFields = show; | ||
component.setShowFields(show); |
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.
Why are we setting this field to true twice?
2ceccec
to
12a95bb
Compare
Issue details