Mobile client overview enhancements

[sedroche]

Adding mobile service related information to the mobile client component on the overview screen.

The component will display:

• Bound/unbound services
• Copy to clipboard component containg sdk info to connect to services
• Mobile client builds status and history.

NOTE: This PR makes use of openshift/origin-web-common#320 and openshift/origin-web-catalog#669

[benjaminapetersen]

@openshift/team-ux-review

[spadgett]

/ok-to-test

[witmicko]

/retest

[beanh66]

@benjaminapetersen @sedroche Same comment @cshinn made on this PR #3022 applies here for the copy icon and action. Please also note an open issue around the tooltip behavior. openshift/console#144

[sedroche]

Just linking this comment here re copy icon -> #3022 (comment)

[benjaminapetersen]

Some observations & a bit of clarifications, thanks!

[benjaminapetersen]

How important is the null, ' ' args to you? I don't think we have done this anywhere else. Fine for dev, but consider removing if not essential (both are optional to the stringify() fn).

[witmicko]

it allows us to pretty print this json, without it we get it inline:

VS

[benjaminapetersen]

Ok, consider a named prettyPrint({obj}) helper function, to make that explicit.

Almost exactly the same:

var getClientConfig = function(mobileClient, serviceConfig, clusterInfo) { 
   return prettyPrintJSON({yourObj})
}

[benjaminapetersen]

This is really only testing for https://, unless you want to go for something more complex, you might just try _.includes(myUrl, 'https://').

[witmicko]

the s is optional, we re looking for http(s)://name.domain/ pattern https://regexr.com/3rcll
any reason why we shouldn't use regex here?

[benjaminapetersen]

Not a prob. I may just suggest (broken record, sorry) you make that explicit with a named function. validProtocol(str) .

[benjaminapetersen]

Prefer _.endsWith(myUrl, '/')

[benjaminapetersen]

You have a couple different ways you are testing & handling adding the trailing /, possible to extract a helper?

[benjaminapetersen]

You already tested for externalUrl above. I wonder if some of this logic can be extracted into a separate named function that will help document the intent.

[benjaminapetersen]

Meaning, there is a reason you are making a pivot & replacing the URL. A named function here would tell me "why".

[benjaminapetersen]

See my other comment about using _.once() to reduce some of this bookkeeping. You won't need to track row.serviceInstancesWatched = true;

var startWatchingServiceClasses = _.once(function() {  watches.push(/* */) });

if(apiObjectChanges) {
   startWatchingServiceClasses();
}

[benjaminapetersen]

Extract to named function?

[benjaminapetersen]

Extract to named function?

[benjaminapetersen]

Extract to named function?

[benjaminapetersen]

Rather than a return false, I'd prob just do:

return !!_.get(row.apiObject, 'metadata.deletionTimestamp') ||
  AuthorizationService.canI(row.mobileClientsVersion, 'delete');

Its a little more idiomatic.

[benjaminapetersen]

aria-hidden="true" on the <i>.

[benjaminapetersen]

aria-hidden="true" on the <i>.