Skip to content
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

add maxVoltageRating to capacitor #629

Merged
merged 4 commits into from
Feb 15, 2025

Conversation

Anshgrover23
Copy link
Contributor

Fixes #622

Copy link

vercel bot commented Feb 15, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
tscircuit-core-benchmarks ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 15, 2025 7:14pm

Copy link
Contributor

@Abse2001 Abse2001 left a comment

Choose a reason for hiding this comment

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

great job!!!

Copy link
Contributor

@seveibar seveibar left a comment

Choose a reason for hiding this comment

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

It’s a bit odd to display the max voltage on the schematic imo

sometimes its done but its actually quite confusing when mixed with simulation.

i think its sometimes important, we could introduce schShowRatings to enable it to be displayed but it should default to off i think

Copy link
Member

@imrishabh18 imrishabh18 left a comment

Choose a reason for hiding this comment

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

I think what @seveibar meant with the schShowRatings prop was, that it should be a parent level prop. Like in board or group

@Anshgrover23
Copy link
Contributor Author

@imrishabh18 no, i dont know about that because he merged my this prop pr tscircuit/props#182 so is this also wrong. or only in core we need to change.

Copy link
Contributor

@seveibar seveibar left a comment

Choose a reason for hiding this comment

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

see comment

@seveibar
Copy link
Contributor

I think what @seveibar meant with the schShowRatings prop was, that it should be a parent level prop. Like in board or group

@imrishabh18 it's not a bad idea for a top level prop but it's fine here too. We will probably have a "showRatings" mode at a subcircuit level at some point because it also makes sense there

Copy link
Contributor

@seveibar seveibar left a comment

Choose a reason for hiding this comment

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

nice work /tip $3

@seveibar seveibar merged commit 5294038 into tscircuit:main Feb 15, 2025
8 checks passed
Copy link

algora-pbc bot commented Feb 15, 2025

🎉🎈 @Anshgrover23 has been awarded $3! 🎈🎊

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

Successfully merging this pull request may close these issues.

Add optional voltage prop for the capacitor component
4 participants