Skip to content
This repository was archived by the owner on Feb 21, 2022. It is now read-only.

Refactor ExampleBrowser#10

Merged
toxicFork merged 1 commit intofirtoz:masterfrom
kachkaev:refactor-browser
Sep 24, 2016
Merged

Refactor ExampleBrowser#10
toxicFork merged 1 commit intofirtoz:masterfrom
kachkaev:refactor-browser

Conversation

@kachkaev
Copy link
Contributor

@kachkaev kachkaev commented Sep 6, 2016

  • navigate between examples with react-router
    (each example has got a slug)
  • extract ExampleViewer into a separate component,
    determine its size using react-sizeme
  • make ExampleBrowser component functional (i.e. pure)

react-three-renderer-routes

- navigate between examples with react-router
  (each example has got a slug)

- extract ExampleViewer into a separate component,
  determine its size using react-sizeme

- make ExampleBrowser component functional (i.e. pure)
"cannon": "^0.6.2",
"react": "~15.3.1",
"react-dom": "^15.3.1",
"react-router": "^3.0.0-alpha.3",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

using 3.0 instead of 2.* because of remix-run/react-router#3611 (comment)

@toxicFork
Copy link
Collaborator

Thank you for the PR, I think I will probably go with firtoz/react-three-renderer#82 but have not had the time to make a decision yet

@kachkaev
Copy link
Contributor Author

kachkaev commented Sep 11, 2016

I agree that using the storybook would be much better. This PR can be a temporal patch until that idea has turned into code. I found it quite inconvenient not to have permalinks for examples while refreshing the app, so mainly fixing just this.

@toxicFork
Copy link
Collaborator

good point!

@kachkaev
Copy link
Contributor Author

ping :–)

@toxicFork
Copy link
Collaborator

Hi, apologies for the lateness, I have been having issues with my internet
provider lately as I have just moved.

I will go to a coffee shop sometime this week with my laptop to merge, test
and push :D

Firtina

On Fri, 23 Sep 2016, 16:07 Alexander Kachkaev, notifications@github.com
wrote:

ping :–)


You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#10 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA0iLctvP83gUjqpOEhOSWCFUuu4E6Zkks5qs-s5gaJpZM4J189M
.

toxicFork added a commit that referenced this pull request Sep 24, 2016
@toxicFork toxicFork merged commit 17517b9 into firtoz:master Sep 24, 2016
toxicFork added a commit that referenced this pull request Sep 24, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants