-
Notifications
You must be signed in to change notification settings - Fork 156
Mark TODOs as comments and add Inputs.search documentation #518
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
Conversation
@@ -23,13 +28,29 @@ Or, as a table: | |||
```js echo | |||
Inputs.table(search) |
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.
TODO: write documentation on why:
Inputs.table(search)
andview(Inputs.table(search))
have the same behavior but- but
Inputs.table(search);
(with the semicolon) doesn't display the table
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 a generic issue, not linked to this particular input
@@ -1,6 +1,8 @@ | |||
# Search input | |||
|
|||
[TODO] check on Hello, Inputs removed below. | |||
[Examples ›](https://observablehq.com/@observablehq/input-search) [API Reference ›](https://github.com/observablehq/inputs/blob/main/README.md#search) |
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.
@allisonhorst should we add the API link right after the h2 header?
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.
Yes let's add it there for now.
@@ -1,6 +1,8 @@ | |||
# Search input | |||
|
|||
[TODO] check on Hello, Inputs removed below. | |||
[Examples ›](https://observablehq.com/@observablehq/input-search) [API Reference ›](https://github.com/observablehq/inputs/blob/main/README.md#search) |
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.
Yes let's add it there for now.
[TODO] check on Hello, Inputs removed below. | ||
[Examples ›](https://observablehq.com/@observablehq/input-search) [API Reference ›](https://github.com/observablehq/inputs/blob/main/README.md#search) | ||
|
||
<!-- [TODO] check on Hello, Inputs removed below. --> |
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.
An update for all of these re: Hello, Inputs: I've tried including content from Hello, Inputs into the Inputs overview doc (#516 ) so that we don't point elsewhere (and especially not a notebook since viewof vs. view, etc.).
@@ -11,7 +13,12 @@ const penguins = FileAttachment("penguins.csv").csv({typed: true}); | |||
``` | |||
|
|||
```js echo | |||
const search = view(Inputs.search(penguins)); | |||
const searchInput = Inputs.search(penguins); |
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 the one we chatted about) we can compare to example in lib/inputs#search
and update for consistency.
No description provided.