-
Notifications
You must be signed in to change notification settings - Fork 3
Feature/compliance chart implementation #231
Conversation
# Conflicts: # package.json
…hub.com/RADAR-CNS/RADAR-Dashboard into feature/compliance-chart-implementation
|
||
import { ChartBaseComponent } from '../chart-base/chart-base.component' | ||
|
||
@Component({ |
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.
For the table, we could have a simpler bar with percentage component that we can "stack" on the table, in this way, there's no need to plot everything in d3 and it would be a lot more generic.
Something that could be used like this:
<md-cell *cdkCellDef="let row">
<bar-component *ngFor="let data of row.compliance" [data]="data"></bar-component>
</md-cell>
@Input() subjectId | ||
data$: Observable<any> | ||
|
||
// constructor (public complianceDataService: CompliancePlotTableService) {} |
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.
avoid leaving behind non-informative comments.
If it's something that you are making a conscious decision to "hack" or think it should be improved in the future use a // TODO:
or // FIXME:
or // NOTE:
) {} | ||
|
||
ngOnInit () { | ||
// this.data$ = this.complianceDataService.getAll() |
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.
avoid leaving behind non-informative comments.
If it's something that you are making a conscious decision to "hack" or think it should be improved in the future use a // TODO:
or // FIXME:
or // NOTE:
data$: Observable<any> | ||
isComplianceLoaded$: Observable<any> | ||
|
||
// constructor (public complianceDataService: CompliancePlotService) {} |
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.
avoid leaving behind non-informative comments.
If it's something that you are making a conscious decision to "hack" or think it should be improved in the future use a // TODO:
or // FIXME:
or // NOTE:
src/assets/data/mock-compliance.json
Outdated
@@ -0,0 +1,311 @@ | |||
{ "dataset": [ |
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.
Use the current data that is in the API for the "ACCELEROMETER" as an example to form this mock data.
{
"header": {
"subjectId": "MRC01",
"sourceId": "00:07:80:1F:17:6B",
"source": "EMPATICA",
"sensor": "ACCELEROMETER",
"descriptiveStatistic": "AVERAGE",
"unit": "G",
"timeFrame": "TEN_SECOND",
"effectiveTimeFrame": {
"startDateTime": "2017-06-16T12:12:30Z",
"endDateTime": "2017-06-16T15:11:20Z"
}
},
"dataset": [
{
"sample": {
"x": -0.35321180555555554,
"y": -0.02673611111111111,
"z": 0.9500868055555556
},
"startDateTime": "2017-06-16T12:12:30Z"
},
{
"sample": {
"x": -0.4702240566037736,
"y": -0.3223270440251572,
"z": 0.4982802672955975
},
"startDateTime": "2017-06-16T12:12:40Z"
},
{
"sample": {
"x": -0.5686910377358491,
"y": -0.5421088836477987,
"z": 0.06436713836477988
},
"startDateTime": "2017-06-16T12:12:50Z"
},
{
"sample": {
"x": -0.7989004629629629,
"y": 0.04933449074074074,
"z": -0.06978202160493827
},
"startDateTime": "2017-06-16T12:13:00Z"
}
]
}
…hub.com/RADAR-CNS/RADAR-Dashboard into feature/compliance-chart-implementation
payload: any | ||
|
||
// TODO: Part of config | ||
complianceKeys = |
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.
I followed the format of taking the keys from the config like in the multi line graph. Is this alright?
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.
Yes. I've added it to the config with a slightly different structure for consistency:
https://radar-dashboard.firebaseio.com/config.json
@@ -20,6 +13,9 @@ import { SubjectDB } from './subject-db' | |||
}) | |||
export class SubjectTableComponent implements OnInit, OnDestroy { | |||
|
|||
// TODO: Fix with service and API | |||
sampleComplianceData = [ { 'type': 'simple', 'value': 0.2 }, { 'type': 'special', 'value': 0.3 }] |
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.
Is this the proper format for the compliance in the table?
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 could be, this LGTM.
) {} | ||
|
||
ngOnInit () { | ||
this.payload = {studyId: this.studyId, keys: this.complianceKeys, timeHoles: this.timeHoles} |
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.
payload
can be scoped to the function or completely removed.
this.store.dispatch(new complianceAction.GetAll({
studyId: this.studyId,
keys: this.complianceKeys,
timeHoles: this.timeHoles
}))
payload: any | ||
|
||
// TODO: Part of config | ||
complianceKeys = |
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.
Remove hard coded value and use the remote config
|
||
case compliance.GET_ALL_SUCCESS: { | ||
const payload = action.payload | ||
const ids = payload |
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.
Given the structure of this data and that it's always going to be completely updated there's no need to create ids
and entities
. Currently, they're redundant.
Suggestion: data
return { keys, values, dates } | ||
} | ||
|
||
private parseTimeHoles (res, multi = false) { |
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.
private parseComplianceData (dataset, keys, timeHoles)
private parseTimeHoles (res, multi = false)
These are either duplicates or very similar to parseMultiValueData
and parseTimeHoles
in https://github.com/RADAR-CNS/RADAR-Dashboard/blob/develop/src/app/components/source-graphs/source-graphs.service.ts#L73
We should abstract these functions to be used in both cases and possibly other cases.
# Conflicts: # package-lock.json # package.json # src/app/app.module.ts # src/app/components/charts/charts.module.ts # src/app/pages/study/study.module.ts
* use createSelector and createFeatureSelector from ngrx * add ComplianceEffects to module
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.
LGTM
No description provided.