-
Notifications
You must be signed in to change notification settings - Fork 328
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
CSV embed using wiki-link transclusion format #1287
base: main
Are you sure you want to change the base?
CSV embed using wiki-link transclusion format #1287
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 6 Skipped Deployments
|
@gradedSystem is attempting to deploy a commit to the Datopian Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add tests.
@olayway added test cases |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some changes are needed, especially to html lib and it's test suite.
Also, please update the README, including instructions on any requirements for using this csv embeds. I suggest trying to use it e.g. in our Flowershow template and document any steps you had to take to make it work.
|
||
test("should return [true, <extension>] for a path with supported file extension", () => { | ||
const filePath = "image.csv"; | ||
expect(isSupportedFileFormat(filePath)).toStrictEqual([false, "csv"]); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is already covered by the test with the same title above.
test("should return [true, <extension>] for a path with supported file extension", () => { | |
const filePath = "image.csv"; | |
expect(isSupportedFileFormat(filePath)).toStrictEqual([false, "csv"]); | |
}); |
expect(serialized).toBe( | ||
'<p><a href="data.csv" class="internal new" download="data.csv">data.csv</a></p>' | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it a regular a tag and not FlatUiTable? Isn't it the aim of this PR?
); | ||
}); | ||
|
||
test("parses a CSV file embed of unsupported file format", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test("parses a CSV file embed of unsupported file format", () => { | |
test("leaves a CSV file embed of unsupported file format as plain text", () => { |
htmlExtensions: [html({ permalinks: ["data.csv"] }) as any], // TODO type fix | ||
}); | ||
expect(serialized).toBe( | ||
'<p><a href="data.csv" class="internal" download="data.csv">data.csv</a></p>' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, why does it include a regular a tag?
htmlExtensions: [html() as any], // TODO type fix | ||
}); | ||
expect(serialized).toBe( | ||
'<p><a href="data.csv" class="internal new" download="data.csv">My CSV File</a></p>' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
???
If anything, alias should be used as table title. But I think you can just ignore it for now.
Fixed the issues with tests @olayway |
Changelog
Added
![[data.csv]]
syntax. CSV files are rendered as tables in markdown.Updated
fromMarkdown.ts
: Modified to handle CSV files by rendering them as tables.html.ts
: Updated to output CSV files as HTML tables when using the![[data.csv]]
syntax.Reference
Issue #1233
cc @olayway