-
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
Inform the user when no projects to select templates from #2399
Inform the user when no projects to select templates from #2399
Conversation
ProjectsService.list().then(function(projectData) { | ||
var projects = _.toArray(projectData.by('metadata.name')); | ||
$scope.projectsExist = _.size(projects) > 0; | ||
watches.push(ProjectsService.watch($scope, function(projectData){ |
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.
We shouldn't watch large project lists.
If you have hundreds or thousands of projects, it's not likely they'll all be deleted anyway while you're on this page :)
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.
They could delete the only project. I could handle no projects in the dialog to avoid the watch.
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.
They could delete the only project.
I'm saying only watch the project list if it's under a certain size like we do in other places.
@@ -103,12 +105,21 @@ angular.module('openshiftConsole') | |||
$scope.catalogItems = items; | |||
dataLoaded(); | |||
})); | |||
ProjectsService.list().then(function(projectData) { | |||
var projects = _.toArray(projectData.by('metadata.name')); | |||
$scope.projectsExist = _.size(projects) > 0; |
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.
We shouldn't need to pay the cost to convert to an array. I'd guess that _.isEmpty
is constant time even for large maps because it only needs to check if at least one item exists.
var projects = projectsData.by('metadata.name');
$scope.projectsExist = !_.isEmpty(projects);
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.
Well, we need to know if there is 0, 1 or more than one. :/
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.
I guess we don't need to know that until the dialog comes up. Will fix.
@@ -3,6 +3,7 @@ | |||
<div class="select-project-for-template"> | |||
<h2>Select from Project</h2> | |||
<ui-select | |||
ng-disabled="$ctrl.numTemplateProjects < 2" |
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.
Instead of a disabled select, is it better to just have static text saying the project name here? That's what we've done elsewhere in the console (e.g. container picker on the view logs tab).
If the select is disabled, I would wonder why. I don't think it's obvious because you only have one project.
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.
I thought the same, but @beanh66 thought it better to be consistent with the select-project component.
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.
@spadgett I was trying to be consistent with what we did for the catalog wizard @dtaylor113 worked on. When there is only 1 project there, the dropdown is disabled. @cshinn any thoughts?
I'm tempted to say just show a "no projects" empty state message in the dialog. It would also be consistent with the "no projects, can't create" message @dtaylor113 added to other dialogs. I'm a little worried these checks might be expensive just to decide whether to hide one link. |
I thought the same but since we cache the projects anyhow, thought maybe this would be the route to go. I'm ok with it either way. @beanh66? |
@jeff-phillips-18 that works, we can do something similar to what Dave has added for the catalog. I believe his says "No Projects Found" for the title with some description text. |
71a047b
to
da46552
Compare
da46552
to
ea89e15
Compare
LGTM @jeff-phillips-18 |
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.
Thanks @jeff-phillips-18
/kind bug |
Automatic merge from submit-queue. |
Also disables the project selection dropdown where there is but one project.
fixes #2396