From 255e3e41ec652f7fd9a6508f3342227a6c3410bf Mon Sep 17 00:00:00 2001 From: Peter Piela Date: Mon, 13 Jun 2016 17:27:24 -0400 Subject: [PATCH] UX improvements to the enroll-node dialog Made the following improvements based on UX review: - Added "X" button to explicity close the dialog - Clicking on the backdrop no longer closes the dialog - The dialog is now draggable from the header - Required form fields are indicated using an asterisk - Added supoort for boolean attributes as True/False selections - Default values when available are used as placeholders - Added input validation for port, address, node name, and deploy_kernel/ramdisk properties - Order driver properties in form based on several rules Change-Id: I4780de2aa49503073bc4d63ec212c4d949b4b2cf --- .../ironic/empty-to-pristine.directive.js | 37 ++ .../enroll-node/enroll-node.controller.js | 171 +++++++- .../admin/ironic/enroll-node/enroll-node.html | 69 ++- .../ironic/enroll-node/enroll-node.service.js | 404 +++++++++++++++--- .../ironic/enroll-node/enroll-node.spec.js | 2 +- .../dashboard/admin/ironic/ironic.module.js | 7 +- .../admin/ironic/modal-draggable.directive.js | 46 ++ 7 files changed, 652 insertions(+), 84 deletions(-) create mode 100644 ironic_ui/static/dashboard/admin/ironic/empty-to-pristine.directive.js create mode 100644 ironic_ui/static/dashboard/admin/ironic/modal-draggable.directive.js diff --git a/ironic_ui/static/dashboard/admin/ironic/empty-to-pristine.directive.js b/ironic_ui/static/dashboard/admin/ironic/empty-to-pristine.directive.js new file mode 100644 index 00000000..2b02d1aa --- /dev/null +++ b/ironic_ui/static/dashboard/admin/ironic/empty-to-pristine.directive.js @@ -0,0 +1,37 @@ +/* + * Copyright 2016 Cray Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +(function () { + 'use strict'; + + angular + .module('horizon.dashboard.admin.ironic') + .directive('emptyToPristine', EmptyToPristine); + + function EmptyToPristine() { + return { + restrict: 'A', + require: 'ngModel', + link: function(scope, elem, attrs, ctrl) { + ctrl.$parsers.push(function(viewValue) { + if (viewValue === "") { + ctrl.$setPristine(); + } + return viewValue; + }); + } + }; + } +})(); diff --git a/ironic_ui/static/dashboard/admin/ironic/enroll-node/enroll-node.controller.js b/ironic_ui/static/dashboard/admin/ironic/enroll-node/enroll-node.controller.js index dfdd4762..55fdb335 100644 --- a/ironic_ui/static/dashboard/admin/ironic/enroll-node/enroll-node.controller.js +++ b/ironic_ui/static/dashboard/admin/ironic/enroll-node/enroll-node.controller.js @@ -29,6 +29,7 @@ 'horizon.app.core.openstack-service-api.ironic', 'horizon.app.core.openstack-service-api.glance', 'horizon.dashboard.admin.ironic.enroll-node.service', + 'horizon.dashboard.admin.ironic.validHostNamePattern', '$log' ]; @@ -37,17 +38,20 @@ ironic, glance, enrollNodeService, + validHostNamePattern, $log) { var ctrl = this; + ctrl.validHostNameRegex = new RegExp(validHostNamePattern); ctrl.drivers = null; ctrl.images = null; ctrl.loadingDriverProperties = false; // Object containing the set of properties associated with the currently // selected driver ctrl.driverProperties = null; + ctrl.driverPropertyGroups = null; - // Paramater object that defines the node to be enrolled + // Parameter object that defines the node to be enrolled ctrl.node = { name: null, driver: null, @@ -64,7 +68,7 @@ } /** - * Get the list of currently active Ironic drivers + * @description Get the list of currently active Ironic drivers * * @return {void} */ @@ -75,7 +79,7 @@ } /** - * Get the list of images from Glance + * @description Get the list of images from Glance * * @return {void} */ @@ -86,7 +90,133 @@ } /** - * Get the properties associated with a specified driver + * @description Check whether a group contains required properties + * + * @param {DriverProperty[]} group - Property group + * @return {boolean} Return true if the group contains required + * properties, false otherwise + */ + function driverPropertyGroupHasRequired(group) { + var hasRequired = false; + for (var i = 0; i < group.length; i++) { + if (group[i].required) { + hasRequired = true; + break; + } + } + return hasRequired; + } + + /** + * @description Convert array of driver property groups to a string + * + * @param {array[]} groups - Array for driver property groups + * @return {string} Output string + */ + function driverPropertyGroupsToString(groups) { + var output = []; + angular.forEach(groups, function(group) { + var groupStr = []; + angular.forEach(group, function(property) { + groupStr.push(property.name); + }); + groupStr = groupStr.join(", "); + output.push(['[', groupStr, ']'].join("")); + }); + output = output.join(", "); + return ['[', output, ']'].join(""); + } + + /** + * @description Comaprison function used to sort driver property groups + * + * @param {DriverProperty[]} group1 - First group + * @param {DriverProperty[]} group2 - Second group + * @return {integer} Return: + * < 0 if group1 should precede group2 in an ascending ordering + * > 0 if group2 should precede group1 + * 0 if group1 and group2 are considered equal from ordering perpsective + */ + function compareDriverPropertyGroups(group1, group2) { + var group1HasRequired = driverPropertyGroupHasRequired(group1); + var group2HasRequired = driverPropertyGroupHasRequired(group2); + + if (group1HasRequired === group2HasRequired) { + if (group1.length === group2.length) { + return group1[0].name.localeCompare(group2[0].name); + } else { + return group1.length - group2.length; + } + } else { + return group1HasRequired ? -1 : 1; + } + return 0; + } + + /** + * @description Order driver properties in the form using the following + * rules: + * + * (1) Properties that are related to one another should occupy adjacent + * locations in the form + * + * (2) Required properties with no dependents should be located at the + * top of the form + * + * @return {void} + */ + ctrl._sortDriverProperties = function() { + // Build dependency graph between driver properties + var graph = new enrollNodeService.Graph(); + + // Create vertices + angular.forEach(ctrl.driverProperties, function(property, name) { + graph.addVertex(name, property); + }); + + /* eslint-disable no-unused-vars */ + + // Create edges + angular.forEach(ctrl.driverProperties, + function(property, name) { + var activators = property.getActivators(); + if (activators) { + angular.forEach(activators, + function(unused, activatorName) { + graph.addEdge(name, activatorName); + }); + } + }); + + /* eslint-enable no-unused-vars */ + + // Perform depth-first-search to find groups of related properties + var groups = []; + graph.dfs( + function(vertexList, components) { + // Sort properties so that those with the largest number of + // immediate dependents are the top of the list + vertexList.sort(function(vertex1, vertex2) { + return vertex2.adjacents.length - vertex1.adjacents.length; + }); + + // Build component and add to list + var component = new Array(vertexList.length); + angular.forEach(vertexList, function(vertex, index) { + component[index] = vertex.data; + }); + components.push(component); + }, + groups); + groups.sort(compareDriverPropertyGroups); + + $log.debug("Found the following property groups: " + + driverPropertyGroupsToString(groups)); + return groups; + }; + + /** + * @description Get the properties associated with a specified driver * * @param {string} driverName - Name of driver * @return {void} @@ -97,6 +227,7 @@ ctrl.loadingDriverProperties = true; ctrl.driverProperties = null; + ctrl.driverPropertyGroups = null; ironic.getDriverProperties(driverName).then(function(response) { ctrl.driverProperties = {}; @@ -106,12 +237,13 @@ desc, ctrl.driverProperties); }); + ctrl.driverPropertyGroups = ctrl._sortDriverProperties(); ctrl.loadingDriverProperties = false; }); }; /** - * Cancel the node enrollment process + * @description Cancel the node enrollment process * * @return {void} */ @@ -120,7 +252,7 @@ }; /** - * Enroll the defined node + * @description Enroll the defined node * * @return {void} */ @@ -130,8 +262,11 @@ $log.debug(name + ", required = " + property.isRequired() + ", active = " + property.isActive() + - ", input value = " + property.inputValue); - if (property.isActive() && property.inputValue) { + ", input-value = " + property.getInputValue() + + ", default-value = " + property.getDefaultValue()); + if (property.isActive() && + property.getInputValue() && + property.getInputValue() !== property.getDefaultValue()) { $log.debug("Setting driver property " + name + " to " + property.inputValue); ctrl.node.driver_info[name] = property.inputValue; @@ -150,7 +285,7 @@ }; /** - * Delete a node property + * @desription Delete a node property * * @param {string} propertyName - Name of the property * @return {void} @@ -160,7 +295,7 @@ }; /** - * Check whether the specified node property already exists + * @description Check whether the specified node property already exists * * @param {string} propertyName - Name of the property * @return {boolean} True if the property already exists, @@ -171,7 +306,7 @@ }; /** - * Delete a node metadata property + * @description Delete a node metadata property * * @param {string} propertyName - Name of the property * @return {void} @@ -181,7 +316,8 @@ }; /** - * Check whether the specified node metadata property already exists + * @description Check whether the specified node metadata property + * already exists * * @param {string} propertyName - Name of the metadata property * @return {boolean} True if the property already exists, @@ -190,5 +326,16 @@ ctrl.checkExtraUnique = function(propertyName) { return !(propertyName in ctrl.node.extra); }; + + /** + * @description Check whether a specified driver property is + * currently active + * + * @param {string} property - Driver property + * @return {boolean} True if the property is active, false otherwise + */ + ctrl.isDriverPropertyActive = function(property) { + return property.isActive(); + }; } })(); diff --git a/ironic_ui/static/dashboard/admin/ironic/enroll-node/enroll-node.html b/ironic_ui/static/dashboard/admin/ironic/enroll-node/enroll-node.html index 3166182a..074732b6 100644 --- a/ironic_ui/static/dashboard/admin/ironic/enroll-node/enroll-node.html +++ b/ironic_ui/static/dashboard/admin/ironic/enroll-node/enroll-node.html @@ -1,4 +1,11 @@ -