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

Do not show binding parameters on the overview page #2326

Merged
merged 1 commit into from Oct 23, 2017

Conversation

jeff-phillips-18
Copy link
Member

@jeff-phillips-18 jeff-phillips-18 commented Oct 20, 2017

See #2323

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 20, 2017
@@ -9,6 +9,7 @@ angular.module('openshiftConsole').component('serviceInstanceBindings', {
],
controllerAs: '$ctrl',
bindings: {
isOverview: '<?',
Copy link
Member

Choose a reason for hiding this comment

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

Maybe call this showParameters and flip the logic?

Copy link
Member Author

Choose a reason for hiding this comment

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

We can but isOverview is already used in the sub-component (serviceBinding) so this is really just a pass thru to that.

Copy link
Member

Choose a reason for hiding this comment

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

I'm OK with it then

@spadgett
Copy link
Member

@jeff-phillips-18 verified the fix works

@spadgett
Copy link
Member

spadgett commented Oct 20, 2017

But we should still look at the margins on the provisioned service page:

openshift web console 2017-10-20 11-10-44

@spadgett
Copy link
Member

Going to leave #2323 open to track the other display issue

@spadgett
Copy link
Member

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 20, 2017
@spadgett spadgett changed the title [WIP] Do not show binding parameters on the overview page Do not show binding parameters on the overview page Oct 20, 2017
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 20, 2017
@spadgett
Copy link
Member

@jeff-phillips-18 dist mismatch

@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 20, 2017
@openshift-merge-robot openshift-merge-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 20, 2017
@jeff-phillips-18
Copy link
Member Author

Rebased to fix dist issues.

@spadgett
Copy link
Member

still some dist problems

@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 23, 2017
@jeff-phillips-18
Copy link
Member Author

Added visual changes for binding parameters as well. See discussion in openshift/origin-web-catalog#518

image

@spadgett
Copy link
Member

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 23, 2017
@openshift-merge-robot
Copy link
Contributor

Automatic merge from submit-queue.

@openshift-merge-robot openshift-merge-robot merged commit d9e3c8a into openshift:master Oct 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants