kdevnel / ynab-absa-csv-convert-js Goto Github PK

Move consts in app.js to the relevant callbacks as suggested in #20

Get the project setup so it is no longer a local install but is in a state that can be collaborated on.

Currently the app only displays the data on the page as stringified JSON.

The next step is to display it in a more visually relevant way.

npm init should be replaced with npm install in README.md

From #20

Add CSS prefixing to Webpack. This will automatically add prefixes like -webkit-appearance to CSS and ensure browser support.

Currently the UI breaks below 540px

Move to using webpack.

Add analytics to hosted app.

Each time I click submit, I get two links with the word test at the bottom of the table.

Ideally, there should be only one link, and it should have a descriptive name.

altere the modifyValues() function to process "OVERSEAS PURCHASE" items

This was mentioned in #20

In order to prepare for integrating webpack and to manage the size of the codebase as it grows let's move to using JS modules to split up the code.

The memo specific logic should be in one place only to improve readability and make the code simpler. I suggest adding memo to the switch.

Suggested in code review issue #20

Save the CSV format string to a file and store on the local filesystem.

This table generator works, but it's not the standard JS way of doing things.
Ideally, you would use document.createElement to generate HTML, this isn't a must-have, but it's a good practice.

What if you would use a configuration array to define what goes into the table and how it's structured. In that case, your outputHTML function would use that configuration to generate both the header and content parts.
An example configuration could be (see The pragmatic programmer
Metaprogramming ):

[
  {
    header: 'Date',
    key: 'Date',
  },
  {
    header: 'Memo',
    key: 'Memo',
  },
]

Originally posted by @bgrgicak in #8 (comment)

From #20

It would be nice to reuse the import file name for the export filename and just add an extension at the end (e.g. input-sample-export.csv).

Issues

• npm init should be replaced with npm install
• Currently, I can upload an invalid file (output-sample.csv), and the page accepts it. Ideally, the app should warn that the file is invalid and refuse to render it.
• Each submit adds an additional download link. There should be just one link (see screenshot 1)
• CSS uses HTML tags for styling, this is a bad practice. CSS classes should be used instead.
• The memo specific logic should be in one place only to improve readability and make the code simpler. I suggest adding memo to the switch.
• Using names like array and string doesn't describe the variable content. The variable content should be clear from reading the name. This can produce long variable names, but that's ok.
• Rename test to something descriptive
• Use JS and a configuration file to render the output table instead of HTML strings. (Tip: this configuration file can also be used for data validation)

Improvements

• Add export unit tests (just for fun 😄)
• Move consts in app.js to the callback.
• Add CSS prefixing to Webpack. This will automatically add prefixes like -webkit-appearance to CSS and ensure browser support.
• It's a good practice not to use anonymous functions, but to move callbacks to proper functions. This would also avoid nested anonymous callbacks like in this callback.
• It would be nice to reuse the import file name for the export filename and just add an extension at the end (e.g. input-sample-export.csv).

Questions

• Why is the CSS bundled inside JS?
• Would it make sense to move the submit callback to a separate file?

Screenshots

Screenshot 1

Rename /dist folder to /docs to support Github Pages natively.

It appears that at some point the code or processing is being duplicated which leads to 2 download buttons being displayed.

From #20

Currently, I can upload an invalid file (output-sample.csv), and the page accepts it. Ideally, the app should warn that the file is invalid and refuse to render it.

Take the final JSON object with all data changes and convert back to a string in CSV format

Move submit event listener callback to a separate module as suggested in #20

At the moment, only the repo has instructions. It would be great to improve the user-facing app on GH Pages to include usage information and so on.

Using names like array and string doesn't describe the variable content. The variable content should be clear from reading the name. This can produce long variable names, but that's ok.

Suggested for improvement in #20

It's not a good practice to add business logic to CSS. It should stay within your code.
If you must have this as before tags there are ways of passing data attributes as before content.

Originally posted by @bgrgicak in #8 (comment)

modifyValues() - Finish "POS PURCHASE" logic branch (split the string and separate into description and memo columns at the right point)

Taken from Review issue #20