-
Notifications
You must be signed in to change notification settings - Fork 52
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
UI Design Stage 2 #39
base: main
Are you sure you want to change the base?
Conversation
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? |
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 ***@***.***> 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
<#29>. What do you
say?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#39?email_source=notifications&email_token=AJXHQZ64FND3EUNIC5JY47TPZ2TY5A5CNFSM4HWMZRYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXK7QKY#issuecomment-500561963>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AJXHQZ3HBOCKE3LNQCIJAV3PZ2TY5ANCNFSM4HWMZRYA>
.
|
The pr is showing changes in the parsing.js. |
I see you made the changes in |
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 ***@***.***> 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
<#39?email_source=notifications&email_token=AJXHQZ62XOCJF6KYSR6TQG3PZ2VSXA5CNFSM4HWMZRYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXLAZUQ#issuecomment-500567250>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AJXHQZY7LK2URA2SQCCPRVTPZ2VSXANCNFSM4HWMZRYA>
.
|
This feature is not implemented yet. I have opened the issue #40 |
@IshaGupta18 thank you! I suppose you solved this elsewhere. If so, may I close it? |
Here's the further development in:
UI Design
Enabling some of the import options with the latest code
This will also close #38
However, I am facing a small problem in the string parsing functionality:
Could anyone suggest why the
CsvParser
object is coming out to be null here? @namangupta01 @Souravirus @jywarren @IgorWilbert Thanks a lot!