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

Disambiguate kinds on other resources by showing group #1478

Merged

Conversation

benjaminapetersen
Copy link
Contributor

@benjaminapetersen benjaminapetersen commented Apr 26, 2017

Getting started on this. Most of the work is in this PR in web-common.

  • bump version in origin-web-common when APIService dedupe is merged
  • show groups for resources in the drop down to make it possible to interact with different resources that happen to have the same name.

I'm not sure if simply listing the group under the kind is extremely clear, but its what we typically do:

screen shot 2017-04-26 at 2 23 10 pm

@benjaminapetersen benjaminapetersen changed the title disambiguate kinds on other resources by showing group [WIP] disambiguate kinds on other resources by showing group Apr 26, 2017
@@ -14,6 +14,7 @@
<ui-select-match placeholder="Choose a resource to list...">{{$select.selected.kind | humanizeKind : true}}</ui-select-match>
<ui-select-choices repeat="kind in kinds | filter : {kind: $select.search} : matchKind | orderBy : 'kind'">
<div ng-bind-html="(kind.kind | humanizeKind : true) | highlight: $select.search"></div>
<small ng-bind-html="kind.group | highlight: $select.search" class="text-muted"></small>
Copy link
Member

Choose a reason for hiding this comment

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

we shouldn't always show this, most of the time it is unnecessary gorp. I would only show it when it is necessary to distinguish between two Kinds that are the same. Similar to what we do in the projects dropdown when we see two projects have the same display name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, am working on that part now

@benjaminapetersen
Copy link
Contributor Author

Updated, now, if I disable the filter on line 103 I can trigger this behavior:

screen shot 2017-04-26 at 3 49 47 pm

Local Subject Access Review is the only item that appears with a group (both).

@jwforres
Copy link
Member

works for me, clean it up so we can merge it

@@ -57,6 +58,24 @@ angular.module('openshiftConsole')
}).toString();
};

var countKinds = function() {
return _.reduce($scope.kinds, function(result, kind) {
Copy link
Member

Choose a reason for hiding this comment

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

We should be able to use _.countBy

https://lodash.com/docs/3.10.1#countBy

@benjaminapetersen benjaminapetersen force-pushed the dedupe-other-resources branch 2 times, most recently from 0da1ad6 to 27ddc69 Compare April 27, 2017 18:12
@benjaminapetersen
Copy link
Contributor Author

ok squashed & updated reduce to countBy.

var counts;
$scope.isDuplicateKind = function(kind) {
if(!counts) {
counts = countKinds();
Copy link
Member

Choose a reason for hiding this comment

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

cant you just do this as a one-liner here using the short form:

counts = _.countBy($scope.kinds, "kind")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updating

@benjaminapetersen benjaminapetersen force-pushed the dedupe-other-resources branch from 27ddc69 to 5071259 Compare May 1, 2017 16:08
@benjaminapetersen benjaminapetersen force-pushed the dedupe-other-resources branch from 5071259 to 1a4c66b Compare May 1, 2017 17:55
@jwforres
Copy link
Member

jwforres commented May 2, 2017

LGTM, just waiting on the bump of origin-web-common

@benjaminapetersen
Copy link
Contributor Author

[merge]

@openshift-bot
Copy link

Evaluated for origin web console merge up to 1a4c66b

@openshift-bot
Copy link

openshift-bot commented May 2, 2017

Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/1310/) (Base Commit: 900019b)

@openshift-bot openshift-bot merged commit 6bc131a into openshift:master May 2, 2017
@benjaminapetersen benjaminapetersen changed the title [WIP] disambiguate kinds on other resources by showing group Disambiguate kinds on other resources by showing group May 3, 2017
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.

4 participants