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.
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
docs: Component Rules #1055
docs: Component Rules #1055
Changes from 1 commit
e8f2a76
1a2c0ea
0899527
ddd7bcc
048adae
646c4ff
6c52c75
cec0436
5c943b8
31c8348
0943739
411c9ee
24aef92
ad5b922
887cba8
f1d984f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
sentence case for headings, please
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.
fixed
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.
@mofojed this isn't true? For instance you can also return hooks.
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.
I did not know you could return hooks from a component. Can you also return primitive values like strings, numbers, and booleans?
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.
@dsmmcken What do you mean return hooks? You can return the result of a hook, but returning the hook itself doesn't make sense (ditto in React). The result of the hook could be a component, list of components,
None
, or a primitive.You can also return primitives though, such as
str
,int
,boolean
.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.
I pushed an update. I'm still not sure if this is saying what we want.
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.
Also, I tested dict and set. They do not open up a panel.
With bool, it opens a panel, but does not render anything (where string and int get rendered).
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 that's correct. I don't know why you'd want to return a
boolean
(since it doesn't get rendered), but React does allow you to do that.Returning a dict or a set could possibly give you an error. There's a little playground at the bottom of the React page where you can play with what are valid returns, we should match that: https://react.dev/learn/your-first-component
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.
Is there a way to add a code playground in our docs? Can we do that in an
md
file?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.
We want to do that, it will not be in the first deployment.
With React Playground it's nice because it all runs client side. We need to have a server running for our playground, so setup is a little more complicated. Probably use a running instance that's shared between all viewers of the docs.
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.
Yeah, that's what I meant, like return [my_state, my_set_state]
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.
@dsmmcken doing
return [my_state, my_set_state]
does not make sense from a component. You can do that from other hooks, but that doesn't make sense in React or in deephaven.ui.You can do
return my_state
. You wouldn't return the setter function.