Skip to content

Export the collection as CSV - #3

Open
remmyothman wants to merge 2 commits into
mahoudeau:mainfrom
remmyothman:csv-export
Open

Export the collection as CSV#3
remmyothman wants to merge 2 commits into
mahoudeau:mainfrom
remmyothman:csv-export

Conversation

@remmyothman

Copy link
Copy Markdown
Contributor

Adds EXPORT AS CSV to the Collection's MORE menu.

  • collectionCsv() lives in collectionview.js (pure, DOM-free), so it is tested like the rest of that module
  • Prefixed with a UTF-8 BOM so Excel reads non-ASCII names like Pokémon correctly
  • Three tests: BOM present, one row per amiibo plus header, and quote escaping

One concern, one PR — as requested.

- collectionCsv() in collectionview.js builds the CSV, prefixed with a
  UTF-8 BOM so spreadsheet apps read non-ASCII names (Pokémon, etc.)
- Kept pure and DOM-free like the rest of that module, so it is tested
- EXPORT AS CSV in the Collection's MORE menu downloads the file
- Three tests: BOM, one row per amiibo + header, quote escaping

@mahoudeau mahoudeau left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The shape is right: pure and DOM-free, so it gets tested like the rest of that module. One fix inline and I'll merge. Worth a fourth test with hasDevice: null to pin it.

The download block is the seventh copy of that create-anchor dance in web/js/. Not yours to fix, I'll pull it into a helper right after this lands.

Comment thread web/js/collectionview.js
item.name ?? '',
item.id ?? '',
item.hasLocal ? 'yes' : 'no',
item.hasDevice ? 'yes' : 'no',

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

hasDevice is three-valued: null means no device has ever been scanned, which is not the same as not being on one, so a fresh visit exports 949 rows claiming On device: no.

Suggested change
item.hasDevice ? 'yes' : 'no',
item.hasDevice === true ? 'yes' : item.hasDevice === false ? 'no' : '',

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