Skip to content

Conversation

@benglin
Copy link
Contributor

@benglin benglin commented Aug 17, 2015

This pull request represents a rebased version of the same work that has already been reviewed, it will be easier to cross-merge to release branch if this needs to be taken in for 0.8.2.

Purpose

This pull request is meant to fix a problem where output ports of a code block node do not completely lined up with their corresponding statements. The defect is being tracked internally as:

  • MAGN-7391 Code block output ports do not align with their corresponding statements

This problem is introduced by slight offset accumulated along the way as output ports are placed in their container ItemsControl. This accumulative error is unavoidable as one double value counts on another previous double value.

To address this problem, instead of having the default StackPanel (inside an ItemsControl) arranging each output port, we now make use of a new Panel derived class: InOutPortPanel. This panel is aware of each PortViewModel (and indirectly, PortModel and PortData) object it contains so it is capable of arranging the ports accurately.

In order not to lose any precision, the vertical offset values are no longer computed in an accumulative way:

double offset = 0.0;
foreach (var port in ports)
{
    // Adding double values accumulates error.
    offset = offset + port.Height;
}

Instead, a new property PortModel.LineIndex (replacing PortModel.VerticalMargin property) is introduced for lossless offset computation. The value of PortModel.LineIndex is assigned when CodeBlockNodeModel generates its output ports (i.e. when it has the knowledge of statement indices too). Error on a port does not get carried over for calculating offset of the next port:

foreach (var port in ports)
{
    double offset = port.LineIndex * port.Height;
}

This is how it looks after the fix:

port-alignment

### Declarations

Check these if you believe they are true

  • The code base is in a better state after this PR
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files

Reviewers

Hi @aparajit-pratap, since you were previously dealing with this, please take a look. Thanks!

FYIs

@riteshchandawar, it's done!

@benglin benglin added the LGTM label Aug 17, 2015
@benglin benglin self-assigned this Aug 17, 2015
benglin added a commit that referenced this pull request Aug 18, 2015
Fixed code block output port alignment issue
@benglin benglin merged commit 990ed80 into DynamoDS:master Aug 18, 2015
@benglin benglin deleted the OutPortAlignment branch August 18, 2015 02:31
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.

1 participant