Split reports code #3185
Split reports code #3185
Conversation
|
|
||
| defaultReportColorsChange(evt) { | ||
| this.props.onChange( | ||
| { default: true }, |
There was a problem hiding this comment.
Should've asked a while ago, are you using any specific Prettier settings (like in a .prettierrc)? Usually we don't have a space padding around brackets like this but I know it's a default in Prettier linting
There was a problem hiding this comment.
Right, so I think that my prettier (I use default with few rules being based on our .editorconfig) and our .eslintrc.json differ as to spaces in this situation. I was used to removing (as this matches how we handle ( and [, etc.).
I also have lines mergin in my editor, and when used it always adds a space between merged lines, resulting in { default: true },. I think I merged those and missed that.
Maybe it's a good moment to add .prettierrc file to our repo? Not to automatically use it right now, but to decide on what's in it for future.
There was a problem hiding this comment.
Maybe it's a good moment to add
.prettierrcfile to our repo? Not to automatically use it right now, but to decide on what's in it for future.
Yeah I agree, just so we're consistent with what our linters are doing for new code :D
| if (resps) { | ||
| reportData[i].data.responseLabels = []; | ||
| for (var j = resps.length - 1; j >= 0; j--) { | ||
| choice = asset.content.choices.find((o) => { |
There was a problem hiding this comment.
wow this section was hard to read before 😅
There was a problem hiding this comment.
hah, still isn't best :D
| if (vals && vals[0] && vals[0][1] && vals[0][1].responses) { | ||
| var respValues = vals[0][1].responses; | ||
| reportData[i].data.responseLabels = []; | ||
| let qGB = asset.content.survey.find((z) => {return z.name === groupBy || z.$autoname === groupBy}); |
There was a problem hiding this comment.
why did these lines not get split into multiple?
| (o.name === r || o.$autoname === r) | ||
| ); | ||
| }); | ||
| reportData[i].data.responseLabels[ind] = (choice && choice.label && choice.label[tnslIndex]) ? choice.label[tnslIndex] : r; |
There was a problem hiding this comment.
same as above (actually a couple more across all files)
| let rowsByKuid = {}; | ||
| let rowsByIdentifier = {}; | ||
| let groupBy = '', | ||
| reportStyles = asset.report_styles, |
There was a problem hiding this comment.
it looked like the linting expanded variable declarations from let x, y into let x; let y;. maybe a missed spot here?
same thing on line 150
Description
Split Reports code into multiple files to make maintaining it easier.
Additional details
This is mostly just moving all
classes defined inreports.es6into their separate files. Also cleaned up linter warnings and defined one new constant.Did it as "nice to have" thing while fixing #3183 (I had to rebuild project multiple times due to Enketo issues so had some time to kill).