Skip to content

Commit

Permalink
REVIEW comments, #32
Browse files Browse the repository at this point in the history
  • Loading branch information
pixelzoom committed Mar 4, 2024
1 parent 97d0b29 commit dfb337f
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 1 deletion.
5 changes: 5 additions & 0 deletions js/common/view/FieldNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,12 @@ type FieldNodeOptions = SelfOptions & NodeOptions;

export default class FieldNode extends Node {

//REVIEW Unnecessary class field. fieldLines is private and unused outside the constructor.
// The field lines are vertical lines spaced evenly along the field, according to the bin width.
private fieldLines: Node[];

//REVIEW Unnecessary class field. fieldBorder is private and unused outside the constructor.
//REVIEW Is this doc incomplete? I found fieldBorder vs fieldBackground to be confusing, especially both are filled.
// The field
private readonly fieldBorder: Node;

Expand Down Expand Up @@ -135,6 +138,8 @@ export default class FieldNode extends Node {
this.setClipArea( maskShape );
}

//REVIEW Document
//REVIEW This function does not access any members (no use of 'this'), so can be moved outside the class definition.
private fieldLinesForBinWidth( binWidth: number ): Node[] {
const totalFieldLines = PDLConstants.MAX_FIELD_DISTANCE / binWidth - 1;
const deltaX = binWidth * PDLConstants.FIELD_WIDTH / PDLConstants.MAX_FIELD_DISTANCE;
Expand Down
1 change: 1 addition & 0 deletions js/common/view/FieldOverlayNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export default class FieldOverlayNode extends Node {

const options = optionize<FieldOverlayNodeOptions, SelfOptions, NodeOptions>()( {}, providedOptions );

//REVIEW move outside of class definition, const NUMBER_OF_TOTAL_DASHES = 30;
const numTotalDashes = 30;
const numDashesToDraw = options.isLeftSide ? 1 : numTotalDashes;

Expand Down
2 changes: 1 addition & 1 deletion js/common/view/FieldSignNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export type FieldSignNodeOptions = SelfOptions & VBoxOptions;
export default class FieldSignNode extends VBox {
public constructor(
fieldProperty: TReadOnlyProperty<Field>,
private readonly headingText: Node,
private readonly headingText: Node, //REVIEW headingText does not need to be a class member, it's unused outside the constructor and flagged by WebStorm.
selectorNode: SelectorNode,
signPostHeight: number,
providedOptions: FieldSignNodeOptions ) {
Expand Down
1 change: 1 addition & 0 deletions js/common/view/HistogramAccordionBox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export default class HistogramAccordionBox extends AccordionBox {
public constructor( content: Node,
providedOptions: HistogramAccordionBoxOptions ) {

//REVIEW move outside class definition, const MARGIN = 8;
const margin = 8;

const options = optionize<HistogramAccordionBoxOptions, SelfOptions, AccordionBoxOptions>()( {
Expand Down

1 comment on commit dfb337f

@pixelzoom
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please sign in to comment.