Conversation
| height={GAME_HEIGHT} | ||
| onMouseMove={this.onMouseMove} | ||
| > | ||
| <GridOverlay show={this.props.showGrid} /> |
There was a problem hiding this comment.
@Bjvanminnen I would expect that passing in this prop would update this component but since it's in a protected div, is there something else I need to do to make this work?
I can log and see that redux state has the right value, but my component does not.
There was a problem hiding this comment.
The short answer is that when you're inside a ProtectedDiv, it's not going to update on redux store changes. Your options are (a) ensure that it has the right state on initial render and never needs to update (presumably not applicable if you want to toggle on/off dynamically) (b) move this outside of the ProtectedDiv (possibly difficult) (c) Make the div not protected (while cleaning up the things that necessitated it being protected) or (d) toggle the state using something like jquery code instead of react (generally not recommended).
(c) is probably the most desirable, but not sure how plausible it is.
|
@Bjvanminnen please take a look - thanks! |
| $("#grid-overlay")[0].style.display = 'none'; | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
Looking at this code again, I'm actually pretty happy with it. The pattern still feels fairly React like. One thing you could do is use refs instead of ids for grid-checkbox. Then you would have something like this.refs.gridCheckbox.className = "fa-check-square-o". For grid-overlay, that may not be be possible because grid-overlay is in a different component.
I think that may be what I actually like least about this - modifying the rendered DOM of another component from this one. Could/should we have a method like this in GridOverlay as well? There I think it could just be
componentWillReceiveProps(nextProps) {
$(ReactDOM.findDOMNode(this)).toggle(nextProps.showGrid)
}There was a problem hiding this comment.
GridOverlay is also in a protected div, so it won't ever receive new props, which is why is has to be modified here. Since they both need to be modified in this component, I just decided to keep them consistent by updating the classname in the same way.
2 participants
Screenshot
At any point during running or reset, the grid can be toggled on and off.Technical decisions - hopefully to reference in the future when there is more time to fix.
Checkbox placement and implementation: At first, I tried putting the checkbox outside of GameButtons because that is a protected div, but GameButtons has some CSS with an after sector that causes GameButtons to have a -18px margin. This takes over click events for buttons below it. Removing the CSS that requires the negative margin breaks other scripts.
The checkbox is now included in GameButtons, but since it's a protected div, it requires jquery to update.
Grid: The grid is inside of VisualizationOverlay which is also protected so it is turned on and off with jquery. Since all other visualizations don't happen while the app is running, I added functionality to allow the grid to show anytime.