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

Series ammeter injects model-type functionality into the SeriesAmmeterNode #1004

Open
matthew-blackman opened this issue Oct 29, 2024 · 0 comments
Assignees

Comments

@matthew-blackman
Copy link

While working on the CODAP wrapper, I found that the SeriesAmmeterNode is performing some functions that seem better suited for the model. See the following code in SeriesAmmeterNode.ts that updates the currentValue based on whether the series ammeter has both start and end connections:

const updateText = () => {
      let readout: string = MathSymbols.NO_VALUE;

      // If it is not an icon and connected at both sides, show the current, otherwise show '-'
      if ( screenView ) {

        const circuit = screenView.model.circuit;
        const startConnection = circuit.getNeighboringVertices( seriesAmmeter.startVertexProperty.get() ).length > 1;
        const endConnection = circuit.getNeighboringVertices( seriesAmmeter.endVertexProperty.get() ).length > 1;

        if ( startConnection && endConnection ) {

          const sign = seriesAmmeter.currentSenseProperty.value === CurrentSense.BACKWARD ? -1 : +1;
          const currentValue = seriesAmmeter.currentReadoutProperty.value === null ? null : sign * seriesAmmeter.currentReadoutProperty.value;
          readout = CCKCUtils.createCurrentReadout( currentValue, false, seriesAmmeter.isAlternate );
        }
        else {
          seriesAmmeter.currentReadoutProperty.value = null;
        }
      }

      stringProperty.value = readout;
    };

@samreid and I discussed this and felt that it was potentially a significant/risky refactor, although we felt it could be worth the effort in the right context. Right now the CODAP wrapper is working well, so this is not pressing, but it makes sense to revisit this when more work is being done in CCK.

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

No branches or pull requests

1 participant