publiclab / simple-data-grapher

Turns CSVs into graphs in a few simple steps; embeds onto other websites!
https://publiclab.github.io/simple-data-grapher/
GNU General Public License v3.0
38 stars 52 forks source link

UI Design Stage 2 #39

Open IshaGupta18 opened 5 years ago

IshaGupta18 commented 5 years ago

Here's the further development in:

image

This will also close #38

However, I am facing a small problem in the string parsing functionality:

image

Could anyone suggest why the CsvParser object is coming out to be null here? @namangupta01 @Souravirus @jywarren @IgorWilbert Thanks a lot!

namangupta01 commented 5 years ago

Hi @IshaGupta18, nice work. I guess we should now slowly start to deprecate the old js code write in the new code as we now have after the merging of this very big pr #29. What do you say?

IshaGupta18 commented 5 years ago

That's what I have done in this PR. However, I am facing a small problem here as you can see. Could you have a look at the code in this PR and see why the CsvParser is being returned as null, although it works fine in handleFileSelectLocal?

On Tue, Jun 11, 2019, 1:01 AM Naman Gupta notifications@github.com wrote:

Hi @IshaGupta18 https://github.com/IshaGupta18, nice work. I guess we should now slowly start to deprecate the old js code write in the new code as we now have after the merging of this very big pr #29 https://github.com/publiclab/simple-data-grapher/pull/29. What do you say?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/publiclab/simple-data-grapher/pull/39?email_source=notifications&email_token=AJXHQZ64FND3EUNIC5JY47TPZ2TY5A5CNFSM4HWMZRYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXK7QKY#issuecomment-500561963, or mute the thread https://github.com/notifications/unsubscribe-auth/AJXHQZ3HBOCKE3LNQCIJAV3PZ2TY5ANCNFSM4HWMZRYA .

namangupta01 commented 5 years ago

The pr is showing changes in the parsing.js.

namangupta01 commented 5 years ago

I see you made the changes in dist/PublicLab.Grapher.js. We don't make changes to files in dist folder. These are generated using Babel and Browserify itself when we run npm run build command which sees the src/ folder for any change and transcompile the es6 code to es5 code in dist/. Please don't make changes to dist/. Make changes to src/ while running npm run install all the changes will be automatically done in dist/ files. I am writing a readme about this.

IshaGupta18 commented 5 years ago

Oh okay, I will do these changes in src and make a fresh PR, although could you please have a look at this bug, since the code is same in both the places? Thank you!

On Tue, Jun 11, 2019, 1:16 AM Naman Gupta notifications@github.com wrote:

I see you made the changes in dist/PublicLab.Grapher.js. We don't make changes to files in dist folder. These are generated using Babel and Browserify itself when we run npm run build command which sees the src/ folder for any change and transcompile the es6 code to es5 code in dist/. Please don't make changes to dist/. Make changes to src/ while running npm run install all the changes will be automatically done in dist/ files. I am writing a readme about this.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/publiclab/simple-data-grapher/pull/39?email_source=notifications&email_token=AJXHQZ62XOCJF6KYSR6TQG3PZ2VSXA5CNFSM4HWMZRYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXLAZUQ#issuecomment-500567250, or mute the thread https://github.com/notifications/unsubscribe-auth/AJXHQZY7LK2URA2SQCCPRVTPZ2VSXANCNFSM4HWMZRYA .

namangupta01 commented 5 years ago

This feature is not implemented yet. I have opened the issue #40

IgorWilbert commented 5 years ago

@IshaGupta18 thank you! I suppose you solved this elsewhere. If so, may I close it?