Skip to content
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

Dependent mapping js #9

Merged
merged 18 commits into from
Mar 9, 2020
Merged

Dependent mapping js #9

merged 18 commits into from
Mar 9, 2020

Conversation

derins
Copy link
Contributor

@derins derins commented Mar 6, 2020

Current issues

  1. No unittests for json2png
  2. png file does not download but display on the screen
  3. user input is always assumed to be optional (not an issue if it's always optional)

Code changes

  1. dependent mapping dict passed to layout.html
  2. json is stored as string and parsed to run as a js object
  3. convert.js updated with jquery to dynamically generate the user input field when converter asks

Documentation changes

none

Validation suggestions

  1. Set up the environment using commands in README.md
  2. run
python app.py
  1. go to localhost:5000 on browser
  2. go to convert tab and upload json file from test_files folder
  3. select png as convertTo option
  4. add custom options to any textbox - or leave them empty

@derins derins requested a review from jongoncalves March 6, 2020 23:00
@jongoncalves
Copy link

@derins you are jumping the gun again. This PR regresses a few issues the previous PR just fixed. Please close this PR, merge master into this branch and re-open the PR. As it stands it contains conflicts and problems.

@derins
Copy link
Contributor Author

derins commented Mar 6, 2020

Got it

@derins derins closed this Mar 6, 2020
@derins derins reopened this Mar 7, 2020
@jongoncalves
Copy link

Looks like you aren't using ajax for this which is causing all this weird JSON manipulation. For now it's ok - we gotta context switch to DEVINE. We'll get back to improvements here later.

@derins derins merged commit 4019c75 into master Mar 9, 2020
@derins
Copy link
Contributor Author

derins commented Mar 9, 2020

I'll add that to the to-do list

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants