-
Notifications
You must be signed in to change notification settings - Fork 230
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 deployment and build config pages more consistent #1032
Conversation
This does give us a little space for doing a more detailed status there, like the metrics for the current deployment and / or the pod donut? doesnt necessarily need to happen in this PR just throwing the idea out |
@jwforres PTAL, ready for review if you're OK with the change. I think it's an improvement. |
@@ -75,8 +75,8 @@ | |||
<div class="row" ng-if="loaded"> | |||
<div class="col-md-12" ng-class="{ 'hide-tabs' : !buildConfig }"> | |||
<uib-tabset> | |||
<uib-tab active="selectedTab.summary"> | |||
<uib-tab-heading>Summary</uib-tab-heading> | |||
<uib-tab active="selectedTab.builds"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will invalidate bookmarked URLs from 3.4, can you just validate if someone comes in with tab=summary that it correctly resets the url to tab=builds
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The link works and shows the first tab, but it doesn't update the param in the URL. I can change that if you like.
<div class="h3"> | ||
<span class="last-status"> | ||
<status-icon status="latestBuild.status.phase"></status-icon> | ||
Last build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm i actually dont care for this change, i dont like "Last" in the context of a running build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I was trying to avoid "Latest deployment" since that has a specific meaning (and not always most recent). I changed it here to be consistent, but I can change it back.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I'll just take out "Last" and say, "Build #12 is running."
<uib-tab active="selectedTab.details"> | ||
<uib-tab-heading>Details</uib-tab-heading> | ||
<uib-tab active="selectedTab.replicaSets"> | ||
<uib-tab-heading>Replica Sets</uib-tab-heading> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how would you feel about "History" instead. I don't really like the tab being called Replica Sets
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The command is also oc rollout history
, so I think "History" is better
sure that works
…On Tue, Dec 13, 2016 at 5:14 PM, Sam Padgett ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In app/views/browse/build-config.html
<#1032>:
> @@ -133,10 +133,10 @@
<!-- Show the latest build and a chart of the recent build duration and status. -->
<div ng-if="builds && (builds | hashSize) > 0" class="build-config-summary">
<!-- Show the latest build status first. -->
- <div class="h3" style="margin-bottom: 0;">
- <span class="latest-build-status">
- <status-icon status="latestBuild.status.phase" style="margin-right: 5px;"></status-icon>
- Latest build
+ <div class="h3">
+ <span class="last-status">
+ <status-icon status="latestBuild.status.phase"></status-icon>
+ Last build
Maybe I'll just take out "Last" and say, "Build #12
<#12> is running."
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1032>, or mute the
thread
<https://github.com/notifications/unsubscribe-auth/ABZk7Svw2v3h4dZbQ3MZ4WWOdUsveH71ks5rHxi0gaJpZM4LMBc->
.
|
nah, as long as the link works I'm ok with it. I suspect it is just falling
back to the first tab as a default.
…On Tue, Dec 13, 2016 at 5:06 PM, Sam Padgett ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In app/views/browse/build-config.html
<#1032>:
> @@ -75,8 +75,8 @@
<div class="row" ng-if="loaded">
<div class="col-md-12" ng-class="{ 'hide-tabs' : !buildConfig }">
<uib-tabset>
- <uib-tab active="selectedTab.summary">
- <uib-tab-heading>Summary</uib-tab-heading>
+ <uib-tab active="selectedTab.builds">
The link works and shows the first tab, but it doesn't update the param in
the URL. I can change that if you like.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1032>, or mute the
thread
<https://github.com/notifications/unsubscribe-auth/ABZk7UH0xpVM32qIZhPwaUsywI9eBXzFks5rHxb2gaJpZM4LMBc->
.
|
f2d685f
to
bf8353b
Compare
@jwforres thanks, updated |
@jwforres updated to use "History" as tab header |
[merge] |
Evaluated for origin web console merge up to 500aded |
Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/853/) (Base Commit: df2b5b3) |
Closes #880
cc @sspeiche @jwforres @ajacobs21e