docs: use_render_queue, use_liveness_scope, use_table_listener docs #1044
+812
−277
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
mofojed
commented
Nov 26, 2024
mofojed
commented
- Adding some more missing hooks documentation
- Closes docs: use_liveness_scope #823, closes deephaven.ui hooks documentation #659
- Adding some more missing hooks documentation - Closes deephaven#823, closes deephaven#659
mofojed
commented
Nov 26, 2024
- Remove the API reference to use_render_queue. The autodoc doesn't work correctly if the function has no parameters - Remove redundant information from the README at the base - Update signature for use_table_listener
margaretkennedy
previously approved these changes
Nov 27, 2024
jnumainville
requested changes
Dec 5, 2024
Co-authored-by: margaretkennedy <[email protected]>
Co-authored-by: margaretkennedy <[email protected]>
- Specify list or dict for the row_data vs row_list methods - Add link to toast - Add warning in the use_table_listener hook intro paragraph
Co-authored-by: margaretkennedy <[email protected]>
- There was already a note in the introduction for the only recommendation we were providing
jjbrosnan
reviewed
Dec 17, 2024
| @@ -0,0 +1,62 @@ | |||
| # use_liveness_scope | |||
There was a problem hiding this comment.
Question for @dsmmcken: When Salmon goes live, will these headings have to change the docs syntax:
---
id: whatever
title: whatever
sidebar_label: ...
---
And if so, is there a transition tool that will make changing them quick?
jjbrosnan
reviewed
Dec 17, 2024
jjbrosnan
reviewed
Dec 17, 2024
jjbrosnan
reviewed
Dec 17, 2024
jjbrosnan
reviewed
Dec 17, 2024
jjbrosnan
reviewed
Dec 17, 2024
|
|
||
| ## Refactoring to avoid liveness scope | ||
|
|
||
| In the above example, we could refactor the component to avoid using `use_liveness_scope` by deriving the table from state instead of setting it directly: |
There was a problem hiding this comment.
Which is the better approach? If this is better than using a liveness scope, then why present the first example at all?
There was a problem hiding this comment.
It's kind of an escape hatch if you need it. Ideally you refactor such that you don't need to use it.
jjbrosnan
reviewed
Dec 17, 2024
jjbrosnan
reviewed
Dec 17, 2024
jjbrosnan
reviewed
Dec 17, 2024
jjbrosnan
reviewed
Dec 17, 2024
There was a problem hiding this comment.
Two overarching comments:
- Link to other docs (
use_state,toast, etc.) every time you mention a method in backticks. - Some images or GIFs would be greatly beneficial to these docs, so I could actually see what happens without having to start up a server myself to run code.
- The empty set seemed to be an odd choice. Default to `None` for the table data hooks.
- Add explanation for sentinel object - Add examples with null value
Co-authored-by: JJ Brosnan <[email protected]>
jnumainville
previously approved these changes
Dec 18, 2024
margaretkennedy
previously approved these changes
Dec 19, 2024
|
@jnumainville I noticed the docs failing when I try to autodoc the |
- it's failing, ticket filed https://deephaven.atlassian.net/browse/DH-18247
mofojed
dismissed stale reviews from margaretkennedy and jnumainville
via
20179a0
margaretkennedy
approved these changes
Jan 3, 2025
jnumainville
approved these changes
Jan 8, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.