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/#806 adding okr module #1494
Conversation
https://www.loom.com/share/0ce36bb3a4924c31944ffb1b163a8ff6 Goal/ Objectives can now be deleted |
<nb-option value="organization">Organization</nb-option> | ||
<nb-option value="team">Team</nb-option> | ||
<nb-option value="individual">Individual</nb-option> | ||
<nb-option value="Organization">Organization</nb-option> |
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.
Please add translation key
async openSetTimeFrame() { | ||
const dialog = this.dialogService.open(EditTimeFrameComponent, { | ||
context: { | ||
type: 'add' |
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.
Maybe make this an enum? Or if it just add/edit convert it into a 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.
will make this an enum
}); | ||
const response = await dialog.onClose.pipe(first()).toPromise(); | ||
if (!!response) { | ||
if (response === 'yes') { |
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.
Same.. maybe this can be a boolean? Let's try to avoid string comparisons like this is possible ?
this.loadPage(); | ||
} | ||
}); | ||
if (response === 'deleted') { |
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.
Same here
@@ -42,6 +45,19 @@ export class KeyResultDetailsComponent implements OnInit, OnDestroy { | |||
new Date(b.createdAt).getTime() - | |||
new Date(a.createdAt).getTime() | |||
); | |||
|
|||
// prevent keyresult updates after deadline | |||
if (this.keyResult.deadline === 'No Custom Deadline') { |
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.
Same here.. string comparisons like these can cause errors in the future and it's usually hard to debug
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.
You can just avoid using string comparisons, otherwise really great work! Thank you
@abinandh15 One last thing, can you please see if your delete confirmation dialog is the same as others in the system? |
One last thing, can you please see if your delete confirmation dialog is the same as others in the system? @rmagon Sorry, I'm not getting your question. Are you asking about this confirmation dialog
|
Please note: we will close your PR without comment if you do not check the boxes above and provide ALL requested information.