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

Web console: log follow link visibility fix #7528

Conversation

benjaminapetersen
Copy link
Contributor

  • Fix log follow link on initial page load
  • Add loading ellipsis while logViewer is pending

@spadgett @jwforres for review!

Fixes #7478

@@ -25,7 +25,7 @@
</div>


<div class="row" ng-if="!chromeless">
<div class="row" ng-if="(!chromeless)">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've gotten into the habit of parens in an ng-if when the var is !var to make it a little more explicit. The ! is easy to miss otherwise. Similar idea to (function() { })() vs function() {}();

@benjaminapetersen
Copy link
Contributor Author

[TEST]

@spadgett spadgett self-assigned this Feb 22, 2016
if($scope.state !== 'logs') {
$scope.$evalAsync(function() {
$scope.state = 'logs';
});
Copy link
Member

Choose a reason for hiding this comment

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

nit: indentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@spadgett spadgett added the lgtm Indicates that a PR is ready to be merged. label Feb 22, 2016
@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/5073/) (Image: devenv-rhel7_3513)

@spadgett
Copy link
Member

Ben asked to remove the merge tag so he can fix the indentation.

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 188b4b5

@spadgett
Copy link
Member

[merge]

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/1487/)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 188b4b5

openshift-bot pushed a commit that referenced this pull request Feb 23, 2016
@openshift-bot openshift-bot merged commit 4683de3 into openshift:master Feb 23, 2016
@benjaminapetersen benjaminapetersen deleted the bpeterse/issue-7478-log-link-load branch February 23, 2016 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/web lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants