Skip to content

Conversation

@sfc-gh-dmatthews
Copy link
Contributor

@sfc-gh-dmatthews sfc-gh-dmatthews commented Dec 8, 2025

📚 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.

Comment on lines +7 to +9
const HOLD_DURATION = 2000; // 2 seconds
const COOLDOWN_DURATION = 1500; // cooldown after trigger
const CIRCUMFERENCE = 2 * Math.PI * 45; // circle circumference
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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() {
}

Comment on lines +96 to +98
return () => {
if (animationFrame) cancelAnimationFrame(animationFrame);
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Comment on lines +23 to +36
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}`;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants