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

MazeThumbnail React component and Storybook entry #15692

Merged
merged 6 commits into from
Jun 12, 2017

Conversation

joshlory
Copy link
Contributor

@joshlory joshlory commented Jun 8, 2017

Extract maze drawMap to a separate file, and re-use it to render a MazeThumbnail React component. Only one skin is currently supported. The thumbnail doesn't (yet) show the start or finish icons.

screen shot 2017-06-07 at 5 21 30 pm

@joshlory joshlory requested a review from Hamms June 8, 2017 00:24
Copy link
Contributor

@Hamms Hamms left a comment

Choose a reason for hiding this comment

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

this looks great! I love seeing code move out of maze.js and into more isolated components.

It does seem a little funky for the start and finish coords to live on subtype but be exclusively controlled by maze; I'd love to (perhaps as another PR) see more of that code moved down to the subtype level.

Maze.start_ = {x: x, y: y};
Maze.finish_ = {x: x, y: y};
Maze.subtype.start = {x: x, y: y};
Maze.subtype.finish = {x: x, y: y};
Copy link
Contributor

Choose a reason for hiding this comment

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

given that start and finish have been moved to subtype, could this also be moved there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's definitely more refactoring to do but I'm tempted to save it for a future PR!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 54bfd79.

@joshlory joshlory merged commit 8d2345e into staging Jun 12, 2017
@joshlory joshlory deleted the extract-maze-draw-map branch June 12, 2017 20:22
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.

None yet

2 participants