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

fix: Schematic capacitor & resistor symbol fix #119

Merged
merged 3 commits into from
Sep 25, 2024

Conversation

imrishabh18
Copy link
Member

No description provided.

@tscircuitbot
Copy link

tscircuitbot commented Sep 24, 2024

Size Report

Bundle Size

  • Base branch size: 340K
  • PR branch size: 340K
  • Difference: 0

Install Size

  • Base branch size: 8.2mb
  • PR branch size: 8.23mb
  • Difference: +.03

Full Howfat Output (PR Branch)

@tscircuit/core@0.0.92 (50 deps, 8.23mb, 882 files, ©undefined)
╭─────────────────────────────────────────┬──────────────┬──────────┬───────┬───────────┬──────────────┬───────────╮
│ Name                                    │ Dependencies │     Size │ Files │ Native    │ License      │ Deprec    │
├─────────────────────────────────────────┼──────────────┼──────────┼───────┼───────────┼──────────────┼───────────┤
│ @lume/kiwi@0.4.4                        │              │  121.6kb │    31 │           │ BSD-3-Clause │           │
├─────────────────────────────────────────┼──────────────┼──────────┼───────┼───────────┼──────────────┼───────────┤
│ @tscircuit/infgrid-ijump-astar@0.0.21   │              │ 241.24kb │    14 │           │              │           │
├─────────────────────────────────────────┼──────────────┼──────────┼───────┼───────────┼──────────────┼───────────┤
│ @tscircuit/math-utils@0.0.4             │              │   20.4kb │    21 │           │              │           │
├─────────────────────────────────────────┼──────────────┼──────────┼───────┼───────────┼──────────────┼───────────┤
│ @tscircuit/props@0.0.65                 │              │ 430.78kb │    29 │           │ ISC          │           │
├─────────────────────────────────────────┼──────────────┼──────────┼───────┼───────────┼──────────────┼───────────┤
│ @tscircuit/soup-util@0.0.31             │            6 │   2.68mb │   156 │           │ ISC          │           │
├─────────────────────────────────────────┼──────────────┼──────────┼───────┼───────────┼──────────────┼───────────┤
│ circuit-json-to-connectivity-map@0.0.17 │            1 │  54.54kb │    27 │           │              │           │
├─────────────────────────────────────────┼──────────────┼──────────┼───────┼───────────┼──────────────┼───────────┤
│ circuit-json@0.0.82                     │            2 │   1.06mb │    66 │           │ ISC          │           │
├─────────────────────────────────────────┼──────────────┼──────────┼───────┼───────────┼──────────────┼───────────┤
│ footprinter@0.0.44                      │           25 │   1.31mb │   214 │           │ ISC          │           │
├─────────────────────────────────────────┼──────────────┼──────────┼───────┼───────────┼──────────────┼───────────┤
│ nanoid@5.0.7                            │              │  10.69kb │    11 │           │ MIT          │           │
├─────────────────────────────────────────┼──────────────┼──────────┼───────┼───────────┼──────────────┼───────────┤
│ performance-now@2.1.0                   │              │  11.08kb │    17 │           │ MIT          │           │
├─────────────────────────────────────────┼──────────────┼──────────┼───────┼───────────┼──────────────┼───────────┤
│ react-reconciler@0.29.2                 │            3 │   1.05mb │    43 │           │ MIT          │           │
├─────────────────────────────────────────┼──────────────┼──────────┼───────┼───────────┼──────────────┼───────────┤
│ react@18.3.1                            │            2 │ 331.04kb │    33 │           │ MIT          │           │
├─────────────────────────────────────────┼──────────────┼──────────┼───────┼───────────┼──────────────┼───────────┤
│ schematic-symbols@0.0.19                │              │ 140.11kb │    40 │           │              │           │
├─────────────────────────────────────────┼──────────────┼──────────┼───────┼───────────┼──────────────┼───────────┤
│ transformation-matrix@2.16.1            │              │ 430.45kb │    56 │           │ MIT          │           │
├─────────────────────────────────────────┼──────────────┼──────────┼───────┼───────────┼──────────────┼───────────┤
│ zod@3.23.8                              │              │ 651.16kb │    50 │           │ MIT          │           │
╰─────────────────────────────────────────┴──────────────┴──────────┴───────┴───────────┴──────────────┴───────────╯

@imrishabh18
Copy link
Member Author

@seveibar Can you review this please? this part of the core is a bit confusing for me. Also, I am not able to add the svg snapshot for you to visualise in test, have tried few things but still the json for the schematic-symbol for capacitor_horz is getting undefined even though I can see that in the build. Will have to look more into it

What this PR does is, that the prop name of symbol_name is matched with the schematic-symbol list and then getting added into the circuit-json.

If this is okay, then will similarly update for other normal-components which are failing in tests. I hope I am making sense

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.

Going to take a closer look in ~1 hr, saw something a but weird

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.

Going to take a closer look in ~1 hr, saw something a but weird

@seveibar
Copy link
Contributor

Ok sorry for the many reviews, the tests are correctly throwing, every symbol has an orientation attached to it e.g. _horz, but generally the approach seems fine to me

@@ -189,7 +189,7 @@ export class NormalComponent<
const { schematicSymbolName } = this.config
if (!schematicSymbolName) return
// TODO switch between horizontal and vertical based on schRotation
const symbol_name = `${this.config.schematicSymbolName}_horz`
const symbol_name = `${this.config.schematicSymbolName}`

const symbol = (symbols as any)[symbol_name] as SchSymbol | undefined
Copy link
Contributor

Choose a reason for hiding this comment

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

the errors in this file are basically just saying you need to go refactor every component that defined the symbol without orientation, which is fine.

@imrishabh18 imrishabh18 merged commit 4c03967 into main Sep 25, 2024
3 checks passed
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