Skip to content

Conversation

@imrishabh18
Copy link
Member

@imrishabh18 imrishabh18 commented Oct 16, 2025

doInitialPcbBoardAutoSize() method was overwriting these calculated values with auto-calculated dimensions based on component placement

@vercel
Copy link

vercel bot commented Oct 16, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
tscircuit-core-benchmarks Ready Ready Preview Comment Oct 16, 2025 0:23am

Comment on lines +1 to +28
import { test, expect } from "bun:test"
import { getTestFixture } from "tests/fixtures/get-test-fixture"

test("board outline dimension is calculated correctly", () => {
const { circuit } = getTestFixture()

circuit.add(
<board
outline={[
{ x: -8, y: -8 },
{ x: 8, y: -8 },
{ x: 8, y: 8 },
{ x: -8, y: 8 },
]}
>
<resistor name="R1" resistance="10k" footprint="0402" />
<capacitor name="C1" capacitance="10uF" footprint="0603" />
</board>,
)

circuit.render()

const pcb_board = circuit.db.pcb_board.list()[0]
expect(pcb_board.width).toBe(16)
expect(pcb_board.height).toBe(16)

expect(circuit).toMatchPcbSnapshot(import.meta.path)
})
Copy link
Contributor

Choose a reason for hiding this comment

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

File naming inconsistency: board-outline-dimension.test.tsx should use .test.ts extension instead of .test.tsx. The .tsx extension is reserved for React components containing JSX, while standard test files should use .ts to maintain consistency with the project's naming conventions.

Spotted by Graphite Agent (based on custom rule: Custom rule)

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

@imrishabh18 imrishabh18 requested a review from seveibar October 16, 2025 12:25
@imrishabh18 imrishabh18 changed the title fix board width height outline fix: board width height when outline is passed Oct 16, 2025
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.

Its a bit ambiguous if width/height are also present if outline is present. The width and height become the bounds. I’d rather have width/height be undefined if an outline is present i think but we could add a “shape” property to board in the future to help clear up ambiguity

@imrishabh18 imrishabh18 merged commit 07bab54 into main Oct 16, 2025
11 checks passed
@imrishabh18 imrishabh18 deleted the fix-board-width-height-outline branch October 16, 2025 16:10
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