Conversation
sfc-gh-dmatthews
requested review from
sfc-gh-kmcgrady and
sfc-gh-tteixeira
as code owners
sfc-gh-dmatthews
requested review from
sfc-gh-bnisco
and removed request for
sfc-gh-kmcgrady and
sfc-gh-tteixeira
| const HOLD_DURATION = 2000; // 2 seconds | ||
| const COOLDOWN_DURATION = 1500; // cooldown after trigger | ||
| const CIRCUMFERENCE = 2 * Math.PI * 45; // circle circumference |
There was a problem hiding this comment.
suggestion: Since these are unchanging consts, it is preferred to define them outside of this function as module level consts. eg:
const HOLD_DURATION = 2000; // 2 seconds
const COOLDOWN_DURATION = 1500; // cooldown after trigger
const CIRCUMFERENCE = 2 * Math.PI * 45; // circle circumference
export default function() {
}| return () => { | ||
| if (animationFrame) cancelAnimationFrame(animationFrame); | ||
| }; |
There was a problem hiding this comment.
suggestion: All of the events that are created must be cleaned up in here too. So that would be all of the mousedown, mouseup, etc events.
| @@ -0,0 +1,89 @@ | |||
| export default function ({ | |||
There was a problem hiding this comment.
question: Do we think this example is worthwhile? I think it wouldn't be ideal if people are building forms on their own in custom components given the great form + input support that already exists in Streamlit. What what specific value do we think this example illustrates for users for custom components?
There was a problem hiding this comment.
It's the custom frontend validation logic that I though people might want. The danger button kind of is a simplified, one-dimensional variation of "frontend validation" so I'm not 100% invested in this particular example if it's not to your tastes.
There was a problem hiding this comment.
Yeah, I think the danger button is probably enough to cover that possibility. With that in mind, I'd prefer to omit this one just so that it doesn't seem like people should reinvent the wheel here. Might also give LLMs the wrong idea!
| function polarToCartesian(angle) { | ||
| const rad = ((angle - 90) * Math.PI) / 180; | ||
| return { | ||
| x: cx + radius * Math.cos(rad), | ||
| y: cy + radius * Math.sin(rad), | ||
| }; | ||
| } | ||
|
|
||
| function describeArc(start, end) { | ||
| const startPoint = polarToCartesian(start); | ||
| const endPoint = polarToCartesian(end); | ||
| const largeArc = end - start > 180 ? 1 : 0; | ||
| return `M ${startPoint.x} ${startPoint.y} A ${radius} ${radius} 0 ${largeArc} 1 ${endPoint.x} ${endPoint.y}`; | ||
| } |
There was a problem hiding this comment.
suggestion: For any functions and/or consts that aren't part of the component lifecycle, it would be better to define those outside of the export default function for readability. It helps separate the business logic from the code necessary to wire something up to the custom component.
This reverts commit 23b1034.
2 participants
📚 Context
This PR adds app source code for embedded apps in the custom components v2 concept docs.
This also carves out an exception in the pre-commit hook so that all example code in the python example source directories use a tab of four spaces. (Code is copied between the app source and docs when writing, and linting is used to make sure it's all well-formatted. I want all the code that we show in the docs to have a consistent indent across languages--four spaces--and it's easier to avoid errors when the docs and the source are exact copies.)
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.