[Kimchi-devel] [PATCHv2 2/3] UI: Support add guest disk
Hongliang Wang
hlwang at linux.vnet.ibm.com
Wed Apr 23 08:46:08 UTC 2014
User level comments:
1. Primary disk is allowed to be detached
I think we should prevent user to detach the primary disk. There should
not a "Detach" Button after the primary disk.
2. Two minor issues in UI
2.1. Be consistent to provider default selection for select menus
We have default values for device type and device bus, but not storage
pool or storage volume. Suggest to automatically select the first option
for them.
2.2. Keep select menu text aligned
Seems there are two base lines for select menus.
Code level comments are inline below.
On 04/22/2014 05:05 PM, lvroyce at linux.vnet.ibm.com wrote:
> From: Royce Lv <lvroyce at linux.vnet.ibm.com>
>
> Extend current cdrom disk add to cdrom disk attachment.
> Also add 'bus' for user to choose from.
>
> Signed-off-by: Royce Lv <lvroyce at linux.vnet.ibm.com>
> ---
> ui/css/theme-default/guest-storage-add.css | 16 +++++
> ui/js/src/kimchi.guest_storage_add.main.js | 110 +++++++++++++++++++++++++++--
> ui/pages/guest-storage-add.html.tmpl | 50 ++++++++++++-
> 3 files changed, 168 insertions(+), 8 deletions(-)
>
> diff --git a/ui/css/theme-default/guest-storage-add.css b/ui/css/theme-default/guest-storage-add.css
> index 4da8389..e0be9c0 100644
> --- a/ui/css/theme-default/guest-storage-add.css
> +++ b/ui/css/theme-default/guest-storage-add.css
> @@ -52,6 +52,22 @@
> cursor: not-allowed;
> }
>
> +.guest-storage-add-wrapper-label, .guest-storage-add-wrapper-controls {
> + display: inline-block;
> +}
> +
> +.guest-storage-add-wrapper-label {
> + height: 38px;
> + line-height: 38px;
> + margin-top: 5px;
> + vertical-align: top;
> + width: 80px;
> +}
> +
> +.guest-storage-add-wrapper-controls {
> + width: 470px;
> +}
> +
> #vm-storage-button-add[disabled] {
> background: #c0c0c0;
> color: #ddd;
> diff --git a/ui/js/src/kimchi.guest_storage_add.main.js b/ui/js/src/kimchi.guest_storage_add.main.js
> index 9e232d7..1f04e55 100644
> --- a/ui/js/src/kimchi.guest_storage_add.main.js
> +++ b/ui/js/src/kimchi.guest_storage_add.main.js
> @@ -18,25 +18,123 @@
> kimchi.guest_storage_add_main = function() {
> var types = [{
> label: 'cdrom',
> - value: 'cdrom'
> + value: 'cdrom',
> + bus: ['ide']
> + },
> + {
> + label: 'disk',
> + value: 'disk',
> + bus: ['virtio', 'ide']
> }];
> kimchi.select('guest-storage-type-list', types);
> + kimchi.select('guest-storage-bus-list', [{label: 'ide', value: 'ide'}]);
>
> var storageAddForm = $('#form-guest-storage-add');
> var submitButton = $('#guest-storage-button-add');
> var nameTextbox = $('input[name="dev"]', storageAddForm);
> var typeTextbox = $('input[name="type"]', storageAddForm);
> + var busTextbox = $('input[name="bus"]', storageAddForm);
> var pathTextbox = $('input[name="path"]', storageAddForm);
> + var poolTextbox = $('input[name="storagepool"]', storageAddForm);
> + var volTextbox = $('input[name="storagevol"]', storageAddForm);
> +
> + typeTextbox.change(function() {
> + $('#guest-storage-bus').selectMenu();
> + var pathObject = {'cdrom': ".path-section", 'disk': '.volume-section'}
> + var selectType = $(this).val();
> + $.each(pathObject, function(type, value) {
> + if(selectType == type){
> + $(value).removeClass('hidden');
> + } else {
> + $(value).addClass('hidden');
> + }
> + });
> +
> + $.each(types, function(index, elem){
> + if (selectType == elem.value) {
> + var buses = new Array();
> + $.each(elem.bus, function (index, elem) {
> + buses[index] = {label: elem, value: elem};
> + });
> + $('#guest-storage-bus').selectMenu("setData", buses);
> + $('#guest-storage-bus-label').text(buses[0].value);
> + $('#guest-storage-bus-type').val(buses[0].value);
Suggest keep consistent on variable naming. e.g.
Text box ID: #*guest-storage-bus-type*-textbox
Label ID: #*guest-storage-bus-type*-label
> + }
> + });
> + });
> +
> + kimchi.listStoragePools(function(result) {
> + var options = [];
> + if (result && result.length) {
> + $.each(result, function(index, storagePool) {
> + if ((storagePool.state=="active") && (storagePool.type !== 'kimchi-iso')) {
> + options.push({
> + label: storagePool.name,
> + value: storagePool.name
> + });
> + }
Seems indent is wrong here. Online code formatting tools can be used.
> + });
> + kimchi.select('guest-add-storage-pool-list', options);
> + }
> + });
> +
> + poolTextbox.change(function() {
> + var options = [];
> + kimchi.listStorageVolumes($(this).val(), function(result) {
> + $('#guest-disk').selectMenu();
> + if (result.length) {
> + $.each(result, function(index, value) {
> + // Only unused volume can be attached
> + if (value.ref_cnt == 0) {
> + options.push({
> + label: value.name,
> + value: value.path
> + });
> + }
> + });
> + }
> + $('#guest-disk').selectMenu("setData", options);
> + });
> + });
> +
> + volTextbox.change(function () {
> + pathTextbox.val(volTextbox.val());
> + pathTextbox.change();
> + });
> +
> + typeTextbox.change(function() {
> + var pathObject = {'cdrom': ".path-section", 'disk': '.volume-section'}
> + var selectType = $(this).val();
> + $.each(pathObject, function(type, value) {
> + if(selectType == type){
> + $(value).removeClass('hidden');
> + } else {
> + $(value).addClass('hidden');
> + }
> + });
> +
> + $.each(types, function(index, elem){
> + if (selectType == elem.value) {
> + var buses = new Array();
> + $.each(elem.bus, function (index, elem) {
> + buses[index] = {label: elem, value: elem};
> + });
> + $('#guest-storage-bus').selectMenu("setData", buses);
> + $('#guest-storage-bus-label').text(buses[0].value);
> + }
> + });
> + });
>
> var submitForm = function(event) {
> - if(submitButton.prop('disabled')) {
> + if (submitButton.prop('disabled')) {
> return false;
> }
>
> var dev = nameTextbox.val();
> var type = typeTextbox.val();
> var path = pathTextbox.val();
> - if(!path || path === '') {
> + var bus = busTextbox.val();
> + if (!path || path === '') {
> return false;
> }
>
> @@ -49,13 +147,13 @@ kimchi.guest_storage_add_main = function() {
> var settings = {
> vm: kimchi.selectedGuest,
> type: type,
> - path: path
> + path: path,
> + bus: bus
> };
>
> if(dev && dev !== '') {
> settings['dev'] = dev;
> }
> -
> kimchi.addVMStorage(settings, function(result) {
> kimchi.window.close();
> kimchi.topic('kimchi/vmCDROMAttached').publish({
> @@ -77,7 +175,7 @@ kimchi.guest_storage_add_main = function() {
>
> storageAddForm.on('submit', submitForm);
> submitButton.on('click', submitForm);
> - pathTextbox.on('input propertychange', function(event) {
> + pathTextbox.on('change input propertychange', function(event) {
> $(submitButton).prop('disabled', $(this).val() === '');
> });
> };
> diff --git a/ui/pages/guest-storage-add.html.tmpl b/ui/pages/guest-storage-add.html.tmpl
> index 71e0610..3f92c83 100644
> --- a/ui/pages/guest-storage-add.html.tmpl
> +++ b/ui/pages/guest-storage-add.html.tmpl
> @@ -41,7 +41,7 @@
> <h2>2. $_("Device Type")</h2>
> <div class="field">
> <p class="text-help">
> - $_("The device type. Currently, only \"cdrom\" is supported.")
> + $_("The device type. Currently, \"cdrom\" and \"disk\" are supported.")
> </p>
> <div class="btn dropdown popable">
> <input id="guest-storage-type" name="type" value="cdrom" type="hidden" />
> @@ -54,7 +54,51 @@
> </div>
> </section>
> <section class="form-section">
> - <h2>3. $_("File Path")</h2>
> + <h2>3. $_("Device Bus")</h2>
> + <div class="field">
> + <div class="btn dropdown popable" id="guest-storage-bus">
> + <input id="guest-storage-bus-type" name="bus" value='ide' type="hidden" />
> + <span class="text" id="guest-storage-bus-label">ide</span>
> + <span class="arrow"></span>
> + <div class="popover">
> + <ul class="select-list" id="guest-storage-bus-list" data-target="guest-storage-bus-type" data-label="guest-storage-bus-label"></ul>
> + </div>
> + </div>
> + </div>
> + </section>
> + <div class="volume-section hidden">
> + <section class="form-section">
> + <h2>4. $_("Storage Pool")</h2>
> + <div class="field storage-field">
> + <p class="text-help">
> + $_("Storage pool which volume located in")</p>
> + <div class="btn dropdown popable">
> + <input value="/storagepools/vg" id="guest-disk-pool" name="storagepool" type="hidden">
> + <span class="text" id="guest-disk-pool-label"></span><span class="arrow"></span>
> + <div class="popover" style="width: 100%">
> + <ul class="select-list" id="guest-add-storage-pool-list" data-target="guest-disk-pool" data-label="guest-disk-pool-label"></ul>
> + </div>
> + </div>
> + </div>
> + </section>
> + <section class="form-section">
> + <h2>5. $_("Storage Volume")</h2>
> + <div class="field storage-field">
> + <p class="text-help">
> + $_("Storage volume to be attached")</p>
> + <div class="btn dropdown popable" id="guest-disk">
> + <input id="guest-disk-vol" name="storagevol" type="hidden">
> + <span class="text" id="guest-disk-vol-label"></span><span class="arrow"></span>
> + <div class="popover" style="width: 100%">
> + <ul class="select-list" id="guest-add-storage-pool-list" data-target="guest-disk-vol" data-label="guest-disk-vol-label"></ul>
> + </div>
> + </div>
> + </div>
> + </section>
> + </div>
> + <div class="path-section">
> + <section class="form-section">
> + <h2>4. $_("File Path")</h2>
> <div class="field">
> <p class="text-help">
> $_("The ISO file path in the server for CDROM.")
> @@ -62,6 +106,8 @@
> <input type="text" class="text" name="path" />
> </div>
> </section>
> + </div>
> + </fieldset>
> </form>
> </div>
> <footer>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20140423/1b2e351c/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 1.png
Type: image/png
Size: 33395 bytes
Desc: not available
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20140423/1b2e351c/attachment.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 2.png
Type: image/png
Size: 27201 bytes
Desc: not available
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20140423/1b2e351c/attachment-0001.png>
More information about the Kimchi-devel
mailing list