Skip to content

New tool JSON>Table/CSV/Excel#1003

Merged
veler merged 5 commits into
DevToys-app:dev/2.0from
sakana280:dev/2.0-json-table
Dec 18, 2023
Merged

New tool JSON>Table/CSV/Excel#1003
veler merged 5 commits into
DevToys-app:dev/2.0from
sakana280:dev/2.0-json-table

Conversation

@sakana280

@sakana280 sakana280 commented Dec 16, 2023

Copy link
Copy Markdown
Contributor

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • New feature or enhancement
  • UI change (please include screenshot!)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Internationalization and localization
  • Other (please describe):

What is the current behavior?

Requests for JSON<>CSV conversion in
#200
#671
#708

What is the new behavior?

  • Converts a JSON array of objects to a grid view that can be copied to clipboard as CSV/TSV/FSV.
  • Flattens nested JSON objects and ignores any nested arrays eg {"a":{"b":1,"c":{"d":2},"e":[3,4]}} becomes {"a_b":1,"a_c_d":2}
  • Smart detection triggered by any JSON. Should this be smarter and only detect on JSON arrays?
  • Any conversion errors (eg not valid JSON, not a JSON array) show as an error message in a single-cell datagrid, and cannot copy to clipboard (button is disabled)
  • Includes GUI and CLI tools.
  • Would resolve JSON to Table #708, and half of JSON < > CSV (or Excel) #200

Other information

image

Possible future ideas:

  • Convert TSV/CSV to JSON:
    • Add a 'paste' button to the datagrid to paste TSV/CSV/CSV-French (auto detect or use the setting?) then convert to json
    • Leave datagrid on right hand side but remove 'input'/'output' labels since the datagrid can be input via the paste mechanism
    • This removes complexity of error handling with a toggle between grid view and text view of the TSV/CSV.
    • Would resolve CSV / XML / Excel to JSON #671 and JSON < > CSV (or Excel) #200
  • Respond to compact mode, showing as vertical layout.

Quality check

Before creating this PR:

  • Did you follow the code style guideline as described in CONTRIBUTING.md
  • Did you build the app and test your changes?
  • Did you check for accessibility? On Windows, you can use Accessibility Insights for this.
  • Did you verify that the change work in Release build configuration
  • Did you verify that all unit tests pass
  • If necessary and if possible, did you verify your changes on:
    • Windows
    • macOS (DevToys 2.0)
    • Linux (DevToys 2.0)

I couldn't build Release, but also couldn't build Release for the base dev/2.0 branch. Error message is:
Could not write to output file 'C:\Programming\DevToys.v2\obj\AnyCPU\DevToys.Business\Release\net7.0\generated\CommunityToolkit.Mvvm.SourceGenerators\CommunityToolkit.Mvvm.SourceGenerators.RelayCommandGenerator\DevToys.Business.ViewModels.ToolPageViewModel.ToggleSelectedMenuItemFavorite.g.cs' -- 'Could not find a part of the path 'C:\Programming\DevToys.v2\obj\AnyCPU\DevToys.Business\Release\net7.0\generated\CommunityToolkit.Mvvm.SourceGenerators\CommunityToolkit.Mvvm.SourceGenerators.RelayCommandGenerator\DevToys.Business.ViewModels.ToolPageViewModel.ToggleSelectedMenuItemFavorite.g.cs'.' DevToys.Business (net7.0) C:\Programming\DevToys.v2\src\app\dev\DevToys.Business\CSC

@veler

veler commented Dec 17, 2023

Copy link
Copy Markdown
Collaborator

A thought on the UI:

This is currently NOT achievable with the API we have, but how about: instead of having a Setting where we chose between CSV, TSV, FSV, we have a drop down button in the command bar to copy as CSV, TSV, FSV, along with "Save as..." (which would suggest CSV...etc).

Something similar to what there is in the text box when the UI gets too small:
image

Except that user could choose between CSV, TSV, FSV.

What do you think @sakana280 ?

Let's no block your PR on this idea, since there's no API to achieve this. It's just something for me to think about for a future PR.

@veler veler left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @sakana280, it looks GREAT in overall :D Thank you so much for this!
Just a few suggestions and we should be good to go.

Comment thread src/app/dev/DevToys.Tools/Tools/Converters/JsonTable/JsonTableConverterGuiTool.cs Outdated
Comment thread src/app/dev/DevToys.Tools/Tools/Converters/JsonTable/JsonTableConverterGuiTool.cs Outdated
Comment thread src/app/dev/DevToys.Tools/Tools/Converters/JsonTable/JsonTableConverter.resx Outdated
Comment thread src/app/dev/DevToys.Tools/Tools/Converters/JsonTable/JsonTableConverter.resx Outdated
Comment thread src/app/dev/DevToys.Tools/Tools/Converters/JsonTable/JsonTableConverter.resx Outdated
Comment thread src/app/dev/DevToys.Tools/Tools/Converters/JsonTable/JsonTableConverter.resx Outdated
Comment thread src/app/dev/DevToys.Tools/Tools/Converters/JsonTable/JsonTableConverter.resx Outdated
Improved naming for CLI tool and UI elements.
Don't overflow large JSON lists.
@sakana280

Copy link
Copy Markdown
Contributor Author

Cheers heaps for the review. I've addressed all the comments (I think?) except with a query about retaining "Excel" in the search terms only.

@veler veler linked an issue Dec 17, 2023 that may be closed by this pull request
@veler veler added the devtoys-v2.0 DevToys v2.0 label Dec 17, 2023
@veler

veler commented Dec 17, 2023

Copy link
Copy Markdown
Collaborator

Hi @sakana280 ,

Thanks for addressing my feedback so quickly. It remains 1 tiny fix to do and we should be all good to go 😍

Cheers !

@veler veler left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello,
It looks great now, thank you so much for your contribution! 😁

@veler
veler merged commit bea8711 into DevToys-app:dev/2.0 Dec 18, 2023
@sakana280

Copy link
Copy Markdown
Contributor Author

Many thanks @veler for helping me navigate the codebase and your speedy review of this PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

devtoys-v2.0 DevToys v2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JSON < > CSV (or Excel)

2 participants