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

Show deployments in Time Travel #3222

Merged
merged 3 commits into from
Jun 12, 2018
Merged

Conversation

fbarl
Copy link
Contributor

@fbarl fbarl commented Jun 12, 2018

Resolves https://github.com/weaveworks/service-ui/issues/2614 (affects only Scope in Weave Cloud).

Reuses a lot of code from <TimeTravelWrapper /> in Service UI - @foot and I had a discussion about converging those two, but that's a separate issue.

Other changes
  • Bump ui-components v0.4.82 -> v0.4.95
  • Use z-index: $layer-front on the whole header

@fbarl fbarl self-assigned this Jun 12, 2018
@fbarl fbarl requested a review from foot June 12, 2018 13:23
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.

It doesn't filter by namespace and the deployment ticks don't match the color of the deployment they represent.

This is really cool. 👌 LGTM.

@@ -91,27 +135,36 @@ class TimeTravelWrapper extends React.Component {
}

function mapStateToProps(state, { params }) {
const scopeState = state.scope || state;

This comment was marked as abuse.

This comment was marked as abuse.

@fbarl
Copy link
Contributor Author

fbarl commented Jun 12, 2018

It doesn't filter by namespace...

Related to https://github.com/weaveworks/service-ui/issues/1358.

...and the deployment ticks don't match the color of the deployment they represent

Not sure how to address that at the moment, but it should probably go into a separate issue :)

@fbarl fbarl merged commit 192f5cc into master Jun 12, 2018
@fbarl fbarl deleted the 2614-show-timeline-deployments branch June 12, 2018 15:47
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