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

Make Time Travel a modular component #2888

Merged
merged 7 commits into from
Oct 17, 2017
Merged

Conversation

fbarl
Copy link
Contributor

@fbarl fbarl commented Oct 11, 2017

Fixes #2893.

Changes
  • Merged multiple Time Travel related components into one TimeTravelComponent, which communicates with the world only through forwarded props and is completely isolated from the Scope Redux state.
  • Started using styled-components library to contain all the Time Travel styles in the component.
  • Also contained all the constants - left some helpers spread as they are likely to be transfered into ui-components repo separately
  • Wrapped the App component into the theme the same way it's done in service-ui repo, to start using more standardized colorset in Scope (note: I tried doing it in main.js and main.dev.js files instead, but that wouldn't work with local proxying so app.js was naturally my second choice).

@fbarl fbarl self-assigned this Oct 11, 2017
@fbarl fbarl force-pushed the make-time-travel-modular-component branch from 0c89171 to 742a8c5 Compare October 16, 2017 12:43
@fbarl fbarl requested a review from jpellizzari October 16, 2017 19:34
@fbarl fbarl changed the title [WIP] Make time travel a modular component Make Time Travel a modular component Oct 16, 2017
@jpellizzari
Copy link
Contributor

Still reviewing this one @fbarl I will try to finish up by the end of the day tomorrow (10/17)

@fbarl
Copy link
Contributor Author

fbarl commented Oct 17, 2017

Still reviewing this one @fbarl I will try to finish up by the end of the day tomorrow (10/17)

No worries @jpellizzari, this PR is not blocking any work at the moment. Forgot to mention what might be obvious, but nothing in Time Travel behaviour should have changed with this PR, it's a pure refactoring one.

Copy link
Contributor

@jpellizzari jpellizzari left a comment

Choose a reason for hiding this comment

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

Nice job!

@fbarl fbarl merged commit cf23c06 into master Oct 17, 2017
@fbarl fbarl deleted the make-time-travel-modular-component branch October 20, 2017 16:38
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.

2 participants