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

Jump to bottom

Support TimeTravel injection #3239

Merged
merged 2 commits into from Jun 29, 2018
Merged

Support TimeTravel injection #3239

merged 2 commits into from Jun 29, 2018

Conversation

fbarl
Copy link
Contributor

@fbarl fbarl commented Jun 26, 2018 •
edited

Resolves #3238.

Not to be merged before https://github.com/weaveworks/service-ui/pull/2797.

Changes
  • Removed all Weave Cloud stuff from the default TimeTravelWrapper in Scope
  • Simplified Time Travel state route transition (merged startTimeTravel action into jumpToTime)
Scenarios to test (for this PR)
  • Standalone Scope - Time Travel shouldn't be displayed (by default)
  • Scope in Weave Cloud in combination with Service UI master (component not injected) - should display basic Time Travel
  • Scope in Weave Cloud in combination with https://github.com/weaveworks/service-ui/pull/2797 (component injected) - should display Time Travel with deployment annotations, as we currently have it

@fbarl fbarl self-assigned this Jun 26, 2018
@fbarl fbarl changed the title [WIP] Support TimeTravel injection Support TimeTravel injection Jun 27, 2018
@fbarl fbarl requested review from foot and guyfedwards June 27, 2018 09:50
foot
foot approved these changes Jun 28, 2018
View reviewed changes
Copy link
Contributor

@foot foot left a comment

Choose a reason for hiding this comment

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

👌 Nicely done. Works in standalone and proxied! Code looks fine. LGTM

@fbarl fbarl merged commit 70d4d3f into master Jun 29, 2018
@fbarl fbarl deleted the 3238-support-time-travel-as-prop branch June 29, 2018 09:01
@fbarl fbarl mentioned this pull request Jun 29, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@foot foot foot approved these changes

@guyfedwards guyfedwards Awaiting requested review from guyfedwards

Assignees

@fbarl fbarl

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@fbarl @foot