(linting): formatted the code to remove ESlint errors: #56
Conversation
|
This is a pretty big PR here! Amazing work! However, it would be best if @rexagod would approve this one. We should wait for his comment here. Thanks a ton! |
|
That's cool. You are welcome |
There was a problem hiding this comment.
This is some really nice work, @osbornetunde. Just a few changes, which I mentioned above, after which we can surely get this merged. Thanks!
package.json
Outdated
| "jquery": "^3.4.1", | ||
| "mocha": "^6.1.4", | ||
| "papaparse": "^4.6.3" | ||
| "papaparse": "^4.6.3", | ||
| "xlsx": "^0.14.3" |
There was a problem hiding this comment.
Could you briefly elaborate on why and where has this been used?
There was a problem hiding this comment.
I noticed the handleFileSelectstring function on line 52-73 in View.js file and the parse method on line 38-53 in CsvParser.js needed papa to parse a csv file but it wasn't installed, so I installed papaparse and imported papa for parsing the csv file.
handleFileSelectstring(val){
console.log("i am at csv string handler");
var csv_string = val.split("\n");
var mat=[];
for (var i=0;i<csv_string.length;i++){
if (csv_string[i]=="" || csv_string[i]==" "){
continue;
}
var dataHash=Papa.parse(csv_string[i],{
dynamicTyping: true,
comments: true
});
mat[i]=dataHash['data'][0];
}
this.csvFile=mat;
let self = this;
document.getElementById(this.uploadButtonId).onclick = (e) => {
console.log("i am uploading");
self.csvParser = new CsvParser(self.csvFile, self.elementId,"csvstring");
};
}
parse(functionParameter){
var count = 0;
Papa.parse(this.csvFile, {
download: true,
dynamicTyping: true,
comments: true,
step: (row) => {
this.csvMatrix[count] = row.data[0];
count += 1;
},
complete: () => {
//calling a function to determine headers for columns
this.startFileProcessing(functionParameter);
}
});
}
"XLSX" was needed to create an excel sheet in the createSheet method in line 297-319 of View.js, so I had to install "xlsx" : "^0.14.3, then import XLSX from it
createSheet(){
var wb = XLSX.utils.book_new();
wb.Props = {
Title: "New Spreadsheet"+this.elementId,
CreatedDate: new Date()
};
wb.SheetNames.push("Sheet"+this.elementId);
var ws_data = this.csvParser.completeCsvMatrixTranspose;
var ws = XLSX.utils.aoa_to_sheet(ws_data);
wb.Sheets["Sheet"+this.elementId] = ws;
var wbout = XLSX.write(wb, {bookType:'xlsx', type: 'binary'});
function s2ab(s) {
var buf = new ArrayBuffer(s.length);
var view = new Uint8Array(buf);
for (var i=0; i<s.length; i++) view[i] = s.charCodeAt(i) & 0xFF;
return buf;
}
saveAs(new Blob([s2ab(wbout)],{type:"application/octet-stream"}), 'newSpreadsheet'+this.elementId+'.xlsx');
}
There was a problem hiding this comment.
Hmm. You've verified this local, right? Did everything work without any issues? Sorry, but I need to make sure we're good here since this seems to be a large refactor.
There was a problem hiding this comment.
How do I verify it on my local? I can find how to run the file on my localhost.
There was a problem hiding this comment.
I suppose @IshaGupta18 can help you set up the dev environment on your local.
There was a problem hiding this comment.
When you clone the repo, do npm install to install the
dependencies, and do npm run build, to see the changes in the main file. This is all I think for the dependencies. @namangupta01 will be able to guide you better on this one. Thanks a lot!
There was a problem hiding this comment.
Oh I very much wrote this msg a few hours ago, I don't know how it wasn't posted. Terribly sorry!
There was a problem hiding this comment.
I ran npm run build, I only see changes in files placed in the dist directory
There was a problem hiding this comment.
Yeah, its the bundled up distributable that gets generated by all the src files and into one huge file in the dist folder. Are the changes as expected when you are running the app?
There was a problem hiding this comment.
Do you mean when I run the app using npm run build? If so, yes, the changes are the ones expected.
Co-Authored-By: Pranshu Srivastava <[email protected]>
Co-Authored-By: Pranshu Srivastava <[email protected]>
-worked on the package.json -removed the all unused"e" -reverted sample.js back to its original state
|
Hi @osbornetunde, wow big pr. Nice work. There are conflicts in some files can you please fix them. And what do you think @jywarren , @rexagod and @IshaGupta18 should be not included dist/ in pr and only admin build and push to dist/ what do you guys think? |
|
@namangupta01 I have resolved the conflicts |
|
@namangupta01 I'm okay with the contributors (non-admins) pushing Also, @IshaGupta18 @namangupta01 Let's get this one merged, so please let @osbornetunde know about the updation in the changes, if any, so we can accelerate things here if the code looks good to you. Thanks! |
|
This does look good to me! Great work here. I think from here on, we can
ignore the dist files!
…On Fri, Jun 28, 2019, 9:38 PM Pranshu Srivastava ***@***.***> wrote:
@namangupta01 <https://github.com/namangupta01> I'm okay with the
contributors (non-admins) pushing dist files into their PR branches.
Also, @IshaGupta18 <https://github.com/IshaGupta18> @namangupta01
<https://github.com/namangupta01> Let's get this one merged, so please
let @osbornetunde <https://github.com/osbornetunde> know about the
updation in the changes, if any, so we can accelerate things here if the
code looks good to you. Thanks!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#56?email_source=notifications&email_token=AJXHQZ5UGRXUTLP4ZVTL533P4YZO3A5CNFSM4H2QFYI2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY2P2XQ#issuecomment-506789214>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AJXHQZZTS2XQGKMF7BOOWITP4YZO3ANCNFSM4H2QFYIQ>
.
|
|
Nice work @osbornetunde, thanks a lot!!! |
|
@IgorWilbert you are welcome |
|
Hello is this PR going to be merge? Because I work in a fix that I would like to PR |
Fixes #50 (<=== Add issue number here)
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
@publiclab/reviewersor the user who published the issue for help, in a comment below@rexagod I'm currentingly having issues linting the transpiled "PublicLab.Grapher.js" in dist directory.
Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
Thanks!