Some comments:
1) Seems there are tabs in this patch.
Please, always use 4 spaces to indentation
2) There are whitespaces in there too
alinefm@alinefm:~/kimchi$ git am ../mail-patches/\[Kimchi-devel\]\
\[PATCH\ V2\]\ Add\ nfs\ server\ and\ target\ UI\ in\ create\ storage\
pool.eml
Applying: Add nfs server and target UI in create storage pool
/home/alinefm/kimchi/.git/rebase-apply/patch:490: trailing whitespace.
});
warning: 1 line adds whitespace errors.
3) About UI
I'd suggest to use a single combo box instead of the select box.
The combo box will display one option to input the server manually and
all servers.
-------------------
| Input new server |
-------------------
| host A |
| host B |
My design just like the attachement picture:
I know what you mean here. But I think if just a single combobox ,it
will make the user confused.
because combobox all have a dropdown arrow. And it stands for this box
is used for choose. If we want to input something.
we should use filter-selectbox. In filter-selectbox, what we input must
in the list of select item. There is not a standard widget which can be
input
and choose. since the UI design does not have a standard rule. I can
not give a clear conclustion which way we should use. But personally
speaking
I like the way I used now, it is more clear.
-------------------
The option "Input new server" will be the default one.
When this option is selected the input box (below the combo box) will
be enabled to user input the server information.
When other option is selected (any host IP) the input box is disabled.
What do you think about it?
On 12/30/2013 07:42 AM, zhoumeina wrote:
> v1-v2 Fix some bug in this patch, make it working and retest
> This patch is working adding a select box in create nfs pool,
> when user don't want to input the server ip or name, he can
> simply select "used the server I have used before" and choose
> a server in a select box, this is more convinent.
>
> Add a new jquery filter-select widget
>
> In order to show server target I did a select box in UI, but sometimes
> maybe the export path number is large. So I made a filter-select
> widget to make user easier to choose the export path he want.
> This is the first jquery widget. And let us make all common widget to
> jquery widget from now on.
>
> Signed-off-by: zhoumeina <zhoumein(a)linux.vnet.ibm.com>
> ---
> ui/css/theme-default/button.css | 11 +--
> ui/css/theme-default/form.css | 6 ++
> ui/css/theme-default/storage.css | 4 +-
> ui/js/Makefile.am | 4 +-
> ui/js/src/kimchi.api.js | 26 ++++++-
> ui/js/src/kimchi.storagepool_add_main.js | 73 +++++++++++++++--
> ui/js/src/kimchi.utils.js | 11 +++
> ui/js/widgets/filter-select.js | 137
> ++++++++++++++++++++++++++++++
> ui/js/widgets/select-menu.js | 86 +++++++++++++++++++
> ui/pages/i18n.html.tmpl | 5 +-
> ui/pages/storagepool-add.html.tmpl | 41 +++++++--
> 11 files changed, 376 insertions(+), 28 deletions(-)
> create mode 100644 ui/js/widgets/filter-select.js
> create mode 100644 ui/js/widgets/select-menu.js
>
> diff --git a/ui/css/theme-default/button.css
> b/ui/css/theme-default/button.css
> index c7ed3f6..f99679a 100644
> --- a/ui/css/theme-default/button.css
> +++ b/ui/css/theme-default/button.css
> @@ -247,17 +247,16 @@
> .btn-select {
> display: inline-block;
> position: relative;
> - height: 38px;
> + height: 30px;
> padding-right: 20px;
> margin: 5px;
> vertical-align: top;
> -webkit-border-radius: 5px;
> -moz-border-radius: 5px;
> - border-radius: 5px;
> - background: #eee;
> + background: #fff;
> box-shadow: -1px -1px 1px #666, 1px 1px 1px #fff, 2px 2px 2px
> rgba(0, 0, 0, .15) inset;
> font-size: 13px;
> - line-height: 38px;
> + line-height: 30px;
> text-align: left;
> cursor: pointer;
> }
> @@ -269,8 +268,8 @@
> .btn-select .arrow {
> position: absolute;
> width: 15px;
> - height: 38px;
> - line-height: 38px;
> + height: 30px;
> + line-height: 30px;
> top: 0;
> right: 5px;
> background: url(../images/theme-default/arrow-down-black.png)
> no-repeat center center;
> diff --git a/ui/css/theme-default/form.css
> b/ui/css/theme-default/form.css
> index c24b277..9db4bba 100644
> --- a/ui/css/theme-default/form.css
> +++ b/ui/css/theme-default/form.css
> @@ -45,3 +45,9 @@
> line-height: 30px;
> padding: 0 5px;
> }
> +
> +.text-help {
> + font-size: 12px;
> + color: #333;
> + margin: 0 0 5px 5px;
> +}
> diff --git a/ui/css/theme-default/storage.css
> b/ui/css/theme-default/storage.css
> index d81dc75..ae89f1b 100644
> --- a/ui/css/theme-default/storage.css
> +++ b/ui/css/theme-default/storage.css
> @@ -529,7 +529,7 @@
> width: 300px;
> display: inline-block;
> vertical-align: top;
> - padding: 5px 5px 5px 20px;
> + padding: 5px 5px 5px 22px;
> }
>
> .storage-type-wrapper-controls input[type="text"] {
> @@ -548,7 +548,7 @@
>
> .storage-type-wrapper-controls > .dropdown {
> margin: 5px 0 0 1px;
> - width: 250px;
> + width: 150px;
> }
>
> .storage-type-wrapper-controls input[type="text"][disabled] {
> diff --git a/ui/js/Makefile.am b/ui/js/Makefile.am
> index 337e369..8dfd4d6 100644
> --- a/ui/js/Makefile.am
> +++ b/ui/js/Makefile.am
> @@ -20,7 +20,7 @@
>
> SUBDIRS = novnc
>
> -EXTRA_DIST = src
> +EXTRA_DIST = src widgets
>
> jsdir = $(datadir)/kimchi/ui/js
>
> @@ -32,7 +32,7 @@ dist_js_DATA = \
> modernizr.custom.2.6.2.min.js \
> $(NULL)
>
> -kimchi.min.js: src/*.js
> +kimchi.min.js: widgets/*.js src/*.js
> cat $(sort $^) > $@
>
> CLEANFILES = kimchi.min.js
> diff --git a/ui/js/src/kimchi.api.js b/ui/js/src/kimchi.api.js
> index fbcf4a2..f6115a8 100644
> --- a/ui/js/src/kimchi.api.js
> +++ b/ui/js/src/kimchi.api.js
> @@ -681,5 +681,29 @@ var kimchi = {
> success : suc,
> error : err
> });
> + },
> +
> + getStorageServers: function(type, suc, err) {
> + var url = kimchi.url + 'storageservers?target_type=' + type;
> + kimchi.requestJSON({
> + url : url,
> + type : 'GET',
> + contentType : 'application/json',
> + dataType : 'json',
> + success : suc,
> + error : err
> + });
> + },
> +
> + getStorageTargets: function(server,type, suc, err) {
> + var url = kimchi.url + 'storageservers/' + server +
> '/storagetargets?target_type=' + type;
> + kimchi.requestJSON({
> + url : url,
> + type : 'GET',
> + contentType : 'application/json',
> + dataType : 'json',
> + success : suc,
> + error : err
> + });
> }
> -};
> +};
> \ No newline at end of file
> diff --git a/ui/js/src/kimchi.storagepool_add_main.js
> b/ui/js/src/kimchi.storagepool_add_main.js
> index b31610a..7955352 100644
> --- a/ui/js/src/kimchi.storagepool_add_main.js
> +++ b/ui/js/src/kimchi.storagepool_add_main.js
> @@ -51,7 +51,72 @@ kimchi.initStorageAddPage = function() {
> });
> $('.host-partition').html(listHtml);
> }
> - kimchi.select('storagePool-list', options);
> + $('#storagePool-list').selectMenu();
> + $('#storagePool-list').selectMenu("setData", options);
> + kimchi.getStorageServers('netfs', function(data) {
> + var serverContent = [];
> + serverContent.push({
> + label : i18n['select_default'],
> + value : ''
> + });
> + if (data.length > 0) {
> + $.each(data, function(index, value) {
> + serverContent.push({
> + label : value,
> + value : value
> + });
> + });
> + }
> + $('#nfs-server-used').selectMenu();
> + $('#nfs-server-used').selectMenu("setData",
serverContent);
> + $('#nfs-server-target').filterselect();
> + $('input[name=nfsServerType]').change(function() {
> + if ($(this).val() === 'input') {
> + $('#nfsServerInputDiv').removeClass('tmpl-html');
> + $('#nfsServerChooseDiv').addClass('tmpl-html');
> + } else {
> + $('#nfsServerInputDiv').addClass('tmpl-html');
> + $('#nfsServerChooseDiv').removeClass('tmpl-html');
> + }
> + });
> + $('#nfsServerSelect').change(function() {
> + $('#nfsserverId').val($(this).val());
> + $('#nfsserverId').trigger("keydown");
> + });
> + $('#nfsserverId').on("keydown",function() {
> + if ($(this).val() !== '' &&
> kimchi.isServer($(this).val())) {
> + $('#nfspathId').removeAttr('disabled');
> + } else {
> + $('#nfspathId').attr('disabled','disabled');
> + }
> + $('#nfs-server-target').filterselect('clear');
> + });
> + $('#nfspathId').focus(function() {
> + var targetContent = [];
> + kimchi.getStorageTargets($('#nfsserverId').val(), 'netfs',
> function(data) {
> + if (data.length > 0) {
> + $.each(data, function(index, value) {
> + targetContent.push({
> + label : value.target,
> + value : value.target
> + });
> + });
> + } else {
> + targetContent.push({
> + label : i18n['msg.no.result'],
> + value : ''
> + });
> + }
> + $('#nfs-server-target').filterselect("setData", targetContent);
> + },function() {
> + targetContent.push({
> + label : i18n['msg.no.result'],
> + value : ''
> + });
> + $('#nfs-server-target').filterselect("setData", targetContent);
> + });
> + });
> + });
> $('#poolType').change(function() {
> if ($(this).val() === 'dir') {
> $('.path-section').removeClass('tmpl-html');
> @@ -88,7 +153,6 @@ kimchi.validateForm = function() {
> } else {
> return kimchi.validateLogicalForm();
> }
> -
> };
>
> kimchi.validateDirForm = function () {
> @@ -116,11 +180,8 @@ kimchi.validateNfsForm = function () {
> kimchi.message.error(i18n['msg.pool.edit.nfspath.blank']);
> return false;
> }
> - var domain =
> "([0-9a-z_!~*'()-]+\.)*([0-9a-z][0-9a-z-]{0,61})?[0-9a-z]\.[a-z]{2,6}"
> - var ip = "(\\d{1,3}\.){3}\\d{1,3}"
> - regex = new RegExp('^' + domain + '|' + ip + '$')
>
> - if(!regex.test(nfsserver)) {
> + if(kimchi.isServer(nfsserver)) {
> kimchi.message.error(i18n['msg.validate.pool.edit.nfsserver']);
> return false;
> }
> diff --git a/ui/js/src/kimchi.utils.js b/ui/js/src/kimchi.utils.js
> index 8af6a11..6439db6 100644
> --- a/ui/js/src/kimchi.utils.js
> +++ b/ui/js/src/kimchi.utils.js
> @@ -163,3 +163,14 @@ kimchi.changetoProperUnit = function(numOrg,
> digits, base) {
>
> kimchi.formatMeasurement = format;
> })();
> +
> +kimchi.isServer = function(server) {
> + var domain =
> "([0-9a-z_!~*'()-]+\.)*([0-9a-z][0-9a-z-]{0,61})?[0-9a-z]\.[a-z]{2,6}"
> + var ip = "(\\d{1,3}\.){3}\\d{1,3}"
> + regex = new RegExp('^' + domain + '|' + ip + '$');
> + if(!regex.test(server)) {
> + return false;
> + } else {
> + return true;
> + }
> +};
> \ No newline at end of file
> diff --git a/ui/js/widgets/filter-select.js
> b/ui/js/widgets/filter-select.js
> new file mode 100644
> index 0000000..02fe5d9
> --- /dev/null
> +++ b/ui/js/widgets/filter-select.js
> @@ -0,0 +1,137 @@
> +/*
> + * Project Kimchi
> + *
> + * Copyright IBM, Corp. 2013
> + *
> + * Authors:
> + * zhoumeina <zhoumein(a)linux.vnet.ibm.com>
> + *
> + * 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($) {
> + $.widget('kimchi.filterselect', {
> + options : {
> + key : 0,
> + value : 0
> + },
> +
> + _create : function() {
> + this.listControl = this.element;
> + this.element.html('');
> + var targetId = this.listControl.data('target');
> + var labelId = this.listControl.data('label');
> + this.target = $('#' + targetId);
> + this.label = $('#' + labelId);
> + this.selectDiv = this.listControl.parent().parent();
> + this.selectDiv.addClass('btn-select dropdown popable');
> + this.listControl.parent().addClass('popover');
> + },
> +
> + _setOption : function(key, value) {
> + },
> +
> + setData : function(options) {
> + this.element.html('');
> + var that = this;
> + var value = this.target.val();
> + var selectedClass = 'active';
> + var itemTag = 'li';
> +
> + that.listControl.on('click', itemTag, function(e) {
> + that.listControl.children().removeClass(selectedClass);
> + that.listControl.addClass(selectedClass);
> + that.target.text($(this).text());
> + var oldValue = that.target.val();
> + var newValue = $(this).data('value');
> + that.target.val(newValue);
> + if (oldValue !== newValue) {
> + that.target.change();
> + }
> + });
> +
> + that.selectDiv.click(function(e) {
> + that.listControl.html('');
> + var items = that._dataList(options);
> + $.each(items, function(index, item) {
> + that.listControl.append(item);
> + })
> + var isOpen = that.selectDiv.hasClass('open');
> + that.selectDiv.removeClass('open');
> + if (!isOpen && items.length > 0) {
> + that.selectDiv.addClass('open');
> + }
> + e.preventDefault();
> + e.stopPropagation();
> + });
> +
> + that.target.keyup(function(event) {
> + that.listControl.html('');
> + var items = that._dataList(options);
> + var temp = 0;
> + $.each(items, function(index, item) {
> + if (item.html().indexOf(that.target.val()) >= 0) {
> + that.listControl.append(item);
> + temp++;
> + }
> + });
> + if (that.listControl.html() === '') {
> + that.listControl.html(i18n['msg.storagepool.unvalid.path']);
> + }
> + if (temp > 0) {
> + that._open();
> + }
> + });
> + },
> +
> + _dataList : function(options) {
> + var item;
> + var itemTag = 'li';
> + var selectedClass = 'active';
> + var items = [];
> + var that = this;
> + $.each(options, function(index, option) {
> + item = $('<' + itemTag + '></' + itemTag +
'>');
> + item.text(option.label);
> + item.data('value', option.value);
> + if (option.value === that.target.val()) {
> + item.addClass(selectedClass);
> + }
> + items.push(item);
> + });
> + return items;
> + },
> +
> + clear : function() {
> + this.target.val("");
> + },
> +
> + _open : function() {
> + var isOpen = this.selectDiv.hasClass('open');
> + if (!isOpen) {
> + this.selectDiv.addClass('open');
> + }
> + },
> +
> + _close : function() {
> + var isOpen = this.selectDiv.hasClass('open');
> + if (isOpen) {
> + this.selectDiv.removeClass('open');
> + }
> + },
> +
> + destroy : function() {
> + // call the base destroy function
> + $.Widget.prototype.destroy.call(this);
> + }
> + });
> +}(jQuery));
> \ No newline at end of file
> diff --git a/ui/js/widgets/select-menu.js b/ui/js/widgets/select-menu.js
> new file mode 100644
> index 0000000..4124096
> --- /dev/null
> +++ b/ui/js/widgets/select-menu.js
> @@ -0,0 +1,86 @@
> +/*
> + * Project Kimchi
> + *
> + * Copyright IBM, Corp. 2013
> + *
> + * Authors:
> + * zhoumeina <zhoumein(a)linux.vnet.ibm.com>
> + *
> + * 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($) {
> + $.widget('kimchi.selectMenu', {
> + options: {
> + key: 0,
> + value: 0
> + },
> +
> + _create : function() {
> + this.listControl = this.element;
> + this.element.html('');
> + var targetId = this.listControl.data('target');
> + var labelId = this.listControl.data('label');
> + this.target = $('#' + targetId);
> + this.label = $('#' + labelId);
> + this.selectDiv = this.listControl.parent().parent();
> + this.selectDiv.addClass('btn-select dropdown popable');
> + this.listControl.parent().addClass('popover');
> + var that = this;
> + },
> +
> + _setOption : function(key,value) {},
> +
> + setData : function (options) {
> + var that = this;
> + var value = this.target.val();
> + var selectedClass = 'active';
> + var itemTag = 'li';
> + $.each(options, function(index, option) {
> + item = $('<' + itemTag + '></' + itemTag +
'>');
> + item.text(option.label);
> + item.data('value', option.value);
> + if(option.value === value) {
> + item.addClass(selectedClass);
> + that.label.text(option.label);
> + }
> + that.listControl.append(item);
> + });
> + that.listControl.on('click', itemTag, function() {
> + that.listControl.children().removeClass(selectedClass);
> + that.listControl.addClass(selectedClass);
> + that.label.text($(this).text());
> + var oldValue = that.target.val();
> + var newValue = $(this).data('value');
> + that.target.val(newValue);
> + if(oldValue !== newValue) {
> + that.target.change();
> + }
> + });
> +
> + that.selectDiv.click(function(e) {
> + var isOpen = that.selectDiv.hasClass('open');
> + that.selectDiv.removeClass('open');
> + if (!isOpen) {
> + that.selectDiv.addClass('open');
> + }
> + e.preventDefault();
> + e.stopPropagation();
> + });
> + },
> +
> + destroy : function() {
> + // call the base destroy function
> + $.Widget.prototype.destroy.call(this);
> + }
> + });
> +}(jQuery));
> \ No newline at end of file
> diff --git a/ui/pages/i18n.html.tmpl b/ui/pages/i18n.html.tmpl
> index c1fc3d1..c95da9b 100644
> --- a/ui/pages/i18n.html.tmpl
> +++ b/ui/pages/i18n.html.tmpl
> @@ -121,7 +121,10 @@ var i18n = {
> 'action_create': "$_("Create")",
> 'msg_warning': "$_("Warning")",
> 'msg.logicalpool.confirm.delete': "$_("It will format your
disk
> and you will loose any data in"
> - " there, are you sure to
> continue? ")"
> + " there, are you sure to
> continue? ")",
> + 'select_default': "$_("Please choose")",
> + 'msg.storagepool.unvalid.path': "$_("This is not a valid
path")",
> + 'msg.no.result' : "$_("No valid result")"
> };
> </script>
> </body>
> diff --git a/ui/pages/storagepool-add.html.tmpl
> b/ui/pages/storagepool-add.html.tmpl
> index 5a2dd45..3a73390 100644
> --- a/ui/pages/storagepool-add.html.tmpl
> +++ b/ui/pages/storagepool-add.html.tmpl
> @@ -27,7 +27,7 @@
> <!DOCTYPE html>
> <html>
> <body>
> - <div class="window" style="width: 600px; height:
600px;">
> + <div class="window" style="width: 800px; height:
650px;">
> <header>
> <h1 class="title">$_("Define a New Storage
Pool")</h1>
> <div class="close">X</div>
> @@ -47,10 +47,10 @@
> <section class="form-section">
> <h2>2. $_("Storage Pool Type")</h2>
> <div class="storage-type-wrapper-controls">
> - <div class="btn dropdown popable">
> + <div>
> <input id="poolType" name="type"
> type="hidden" value="dir"/>
> <span class="text"
> id="pool-type-label"></span><span
class="arrow"></span>
> - <div class="popover" style="width:
100%">
> + <div style="width: 100%">
> <ul class="select-list"
> id="storagePool-list" data-target="poolType"
> data-label="pool-type-label">
> </ul>
> </div>
> @@ -74,22 +74,43 @@
> <section class="form-section">
> <h2>3. $_("NFS server IP")</h2>
> <div class="field">
> + <input type="radio"
id="nfsServerInput"
> value="input" name="nfsServerType" checked="true">
> + <label>$_("I want to input the server
> myself.")</label>
> + <input type="radio"
id="nfsServerChoose"
> value="choose" name="nfsServerType">
> + <label>$_("I want to choose a server I
> used before.")</label>
> + </div>
> + <div id="nfsServerInputDiv"
class="field">
> <p class="text-help">
> $_("NFS server IP or hostname. It
> should not be empty.")</p>
> <input id="nfsserverId"
type="text"
> class="text"
> style="width: 300px">
> </div>
> + <div id="nfsServerChooseDiv" class="field
> tmpl-html" style="overflow:visible">
> + <p class="text-help">
> + $_("Please choose the nfs server you
> want to create storage pool.")</p>
> + <div style="width: 285px">
> + <input id="nfsServerSelect"
> type="hidden"/>
> + <span class="text"
> id="nfs-server-label"></span><span
class="arrow"></span>
> + <div style="width: 100%">
> + <ul class="select-list"
> id="nfs-server-used" data-target="nfsServerSelect"
> data-label="nfs-server-label">
> + </ul>
> + </div>
> + </div>
> + </div>
> </section>
> <section class="form-section">
> <h2>4. $_("NFS Path")</h2>
> - <div class="field">
> + <div class="field"
style="overflow:visible">
> <p class="text-help">$_("The nfs
> exported path on nfs server")</p>
> - <input id="nfspathId" type="text"
> class="text"
> - style="width: 300px">
> - <input type="hidden"
id="localpathId"
> class="text"
> - value="none">
> - </div>
> - <div class="clear"></div>
> + <div style="width: 285px">
> + <input id="nfspathId"
class="text"
> disabled="disabled" style="width:293px;"/>
> + <span class="text"
> id="nfs-target-label"></span><span
class="arrow"></span>
> + <div style="width: 100%">
> + <ul class="select-list"
> id="nfs-server-target" data-target="nfspathId"
> data-label="nfs-target-label">
> + </ul>
> + </div>
> + </div>
> + </div>
> </section>
> </div>
> <div class="logical-section tmpl-html">