Refactor Label model #254
Refactor Label model #254
Conversation
Usage of `label.name` is removed from: * Label filter bar component, search
src/app/core/models/label.model.ts
Outdated
| readonly category: string; | ||
| readonly name: string; | ||
| readonly formattedName: string; // 'category'.'name' (e.g. severity.Low) if a category exists or 'name' if the category does not exist. | ||
| readonly formattedName: string; |
There was a problem hiding this comment.
Wondering if we should refactor all instances of label.formattedName to just label.name to prevent confusion since we now aren't doing any formatting of the name.
There was a problem hiding this comment.
I have changed label.formattedName to label.name. Do let me know if there are any other issues!
| @@ -8,7 +8,7 @@ import { LabelService } from '../../../core/services/label.service'; | |||
| styleUrls: ['./issue-pr-card-labels.component.css'] | |||
| }) | |||
| export class IssuePrCardLabelsComponent { | |||
| @Input() labels: Label[]; | |||
| @Input() labelSet: Set<Label>; | |||
| @Input() labels: GithubLabel[]; | |||
There was a problem hiding this comment.
Don't think this is needed now that the name property of a Label is present.
There was a problem hiding this comment.
Can I clarify, do you mean that the type annotation for the field labels can be left as Label[]? The reason I changed the type annotation is because in the only place that this component is used, in issues-pr-card.component.html:
<mat-card-content>
<app-issue-pr-card-milestone [milestone]="issue.milestone"></app-issue-pr-card-milestone>
<app-issue-pr-card-labels [labels]="issue.githubLabels" [labelSet]="dropdownFilter?.hiddenLabels"></app-issue-pr-card-labels>
</mat-card-content>The labels field is given issue.githubLabels, which is of type GithubLabel[].
luminousleek
dismissed
their stale review
Mistakenly clicked approve, should have requested changes.
Summary:
Fixes #229
Type of change:
Changes Made:
As also discussed in #230, there isn't a need for separation of label full name into label name and label category.
Label::nameandLabel::categoryare completely removed. Components now make use ofLabel::formattedNameinstead.With this change,
Label::formattedNameis the label's full name instead of (previously) only the first two parts, hence filters would work as expected regardless of number of periods.in the label name.One consequence of this change is that dropdown filter searching now would search in "category" part of labels as well. For example, when searching "cat" in the dropdown for this repo:
All the labels with name
category.*will be included. Previously, with the separation of label full name into name and category, onlyduplicatelabel is included. I believe that now with the removal of the aforementioned separation, the inclusion ofcategory.*is necessary.While searching for the use of
Label::name, I found out that theIssuePrCardLabelsComponenthas wrong type annotations. I hence have made correction to the type annotations.Screenshots:
Proposed Commit Message:
Checklist: