-
Notifications
You must be signed in to change notification settings - Fork 14
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
Fixed bar chart color display. Specified variable types. #1462 #1527
Fixed bar chart color display. Specified variable types. #1462 #1527
Conversation
…ColorIndicators and conditionsFilterOptions(for correct display order). Updated variable types previously designated as 'any'.
...pp/features/dashboard/home/components/enrollment-over-time/enrollment-over-time.component.ts
Outdated
Show resolved
Hide resolved
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.
The code looks good, you added 2 blank lines at some places instead of 1 and uneven space around =
. Can you add a prettier extension, it will help you fix this.
frontend/projects/upgrade/src/app/core/experiments/store/experiments.model.ts
Show resolved
Hide resolved
frontend/projects/upgrade/src/app/core/experiments/store/experiments.model.ts
Show resolved
Hide resolved
frontend/projects/upgrade/src/app/core/experiments/store/experiments.model.ts
Show resolved
Hide resolved
.../features/dashboard/home/components/enrollment-over-time/enrollment-over-time.component.html
Outdated
Show resolved
Hide resolved
...pp/features/dashboard/home/components/enrollment-over-time/enrollment-over-time.component.ts
Outdated
Show resolved
Hide resolved
...pp/features/dashboard/home/components/enrollment-over-time/enrollment-over-time.component.ts
Show resolved
Hide resolved
...pp/features/dashboard/home/components/enrollment-over-time/enrollment-over-time.component.ts
Show resolved
Hide resolved
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 check small changes requested.
@@ -91,7 +96,9 @@ export class EnrollmentOverTimeComponent implements OnChanges, OnInit, OnDestroy | |||
this.experiment.conditions.forEach((condition) => { | |||
this.conditionsFilterOptions.push({ code: condition.conditionCode, id: condition.id }); | |||
this.selectedCondition.push(condition.id); | |||
this.conditionsFilterOptions.sort((a, b) => a.code.localeCompare(b.code)); |
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.
The sort is done in every loop. This statement can be moved outside of the loop
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.
Moved sorting outside the loop
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.
everything looks good to me.
Fixes bar chart color display. Sorting the displaying graphData, graphColorIndicators and conditionsFilterOptions(for correct display order). Gave variable types previously designated as 'any'.