Minimum Width Warning

[voltaaage]

Builds the feature for #30

Goals:

• [x] Change the existing language of 'length' to represent width. (The board's width changes with each additional strip)
• [x] Display a warning for the user when the width of the board goes beyond a pre-defined width.
• [x] Refactor to make sure it is coded in a non redundant style while abiding by the React philosophy of componentizing and reacting to state/prop changes

Considerations

• We are not considering dynamic widths at this time. A min width of 13 is requested at this time until we have more input from the client.
• The UI of adding/removing/updating strips needs to be updated to allow for easier refinement of the overall board width. Since this is a much larger project that needs to be more throroughly thought out, we are opting for a simple warning to display to the user that they have achieved the maximum width. However, we are not blocking them from adding more strips at this time.

[brian-slate]

@voltaaage is this ready to review?

[voltaaage]

If you have some advice, I'm open to any feedback you have. I'm planning to go through it and refactor things once I'm back from my weekend trip On Sat, Oct 8, 2016 at 6:17 PM Brian Herold notifications@github.com wrote:

@voltaaage https://github.com/voltaaage is this ready to review?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pcww/builder/pull/53#issuecomment-252457784, or mute the thread https://github.com/notifications/unsubscribe-auth/AGyCRVNAyo6vr9ZzGwIwBnThtTyICuvGks5qyECegaJpZM4KPmAC .

[voltaaage]

Ready for review :tennis:

@trezy @bmherold

[voltaaage]

Where do you see {calculated}? I'm pulling in the current width in the warning

[voltaaage]

Found the {calculated} piece: in BoardModel.json:

  "length": "{calculated}",

export default class Wizard extends React.Component {
  getDimensions () {
    let board = this.props.board

    return `${board.get('width')}" x ${board.get('length')}"`
  }

This breaks because I removed the update length method. I'll have to throw it back in.

[voltaaage]

{calculated} issue fixed!!

[trezy]

• [ ] {calculated} is almost fixed. It still shows calculated when you first load up a board.
• [x] I can no l longer change the board length. Adjusting the slider causes the dimensions in the overlay to be updated, but it doesn't update the board length anymore.
• [ ] The remove strip links aren't disabled until the board reaches ~9". I thought it was set at 13"?

[voltaaage]

@trezy I'm not disabling any controls at minimum width - just showing a warning like we discussed

The {calculated} issue is related to the length rendering. It seems to be coupled fairly tightly in the virtual board and I'm wondering if it's worthwhile to switch the logic over to use change how we're using width vs length - or just keep using length as is.

[voltaaage]

Current Issues:

• [ ] Weird CPU usage spikes while using the app - unsure if this is related to my changes yet. It appears to be remedied when I have a value for length set.
• [ ] Initial state of the board has an undefined length. This seems to come in as "{calculated}" and the only reference to this string that I could find is in the boardDataModel.json files.
• Attempted to update those files to no success.
• Also trying to initialize it in the board model - doesn't like it.
• Set it in the constructor for Builder (definitely not the best place) which worked... but now it doesn't
• [ ] Found the disabled toggle - I can hook in the minimum width logic if that is what we want. Right now, it just looks at minimum number of strips. I was under the understanding that we just want to show the warning at the bottom of the Wizard.

[voltaaage]

Currently blocked by the #2 and #3 of the issues in the comment above. I think this PR relies on updating the API (flipping length and width in the response) & a clarification of the requirements in #3.

[brian-slate]

I added a temporary swap of length/width in the backbone model. This looks good. Once @trezy verifies as well we should merge this in and then make new tickets for anything outstanding.