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

Refactor ExampleBrowser #10

Merged
merged 1 commit into from
Sep 24, 2016
Merged

Refactor ExampleBrowser #10

merged 1 commit into from
Sep 24, 2016

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)
@@ -20,6 +20,8 @@
"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, [email protected]
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