On 2/3/2011 8:41 AM, Adam Young wrote:
NACK. Mostly good, but not sure I agree 100%. Line level Undo we very
specific for the multi values. Undo should be for individuals, not for
the overall.
I realize that this makes the logic a little bit harder if you want to,
say, abandon your changes on phonen numbers, but keep them for Title, it
is hard to get the undo just right.
So: Multi values should have an "undo all" in addition to line level undo.
Attached is an updated patch. The line-level undo has been added.
I'd like to leave the "line-out" approach in there for removed entries
as well. A user can always repurpose a line, so there undo/redo will be
valuable at the line level. For straight delete, I think it is valuable
for the user to see the original value.
Also, it looks like the code for "create_remove_link" is still in
IPA.details_field. I'm guessing that this is dead code that should be
removed. At a minimum, it should be moved to the new widget.
Line-out removal has been added as well. Please see the new patch
description. Thanks!
--
Endi S. Dewata
From 055f18eba826a46ec0d182316808b294db1fe9f6 Mon Sep 17 00:00:00 2001
From: Endi S. Dewata <edew...@redhat.com>
Date: Wed, 2 Feb 2011 16:18:35 -0600
Subject: [PATCH] Added multi-valued text widget.
A multi-valued text widget has been created to replace the old
IPA.details_field. The old code was designed to handle all data
types, and it uses one <dd> tag for each value, so the code is
still incomplete and complex. The new code was designed to handle
only multi-valued text attributes, and it uses one <dd> tag for
all values, so it's easier to maintain. There are already other
widgets that can be used to handle other data types.
The new code supports line-level undo and line-out for removal
like the old code, but there are some changes:
- Undoing a newly added line will remove the entire line.
- Editing the value of a removed line will cancel the removal.
- It provides 'undo all' link to reset the entire attribute.
The old code will be cleaned up in a subsequent patch.
---
install/ui/details.js | 15 +-
install/ui/ipa.css | 4 +-
install/ui/test/details_tests.js | 46 ++----
install/ui/test/widget_tests.js | 34 ++++-
install/ui/user.js | 10 +-
install/ui/widget.js | 302 +++++++++++++++++++++++++++++++++++++-
6 files changed, 360 insertions(+), 51 deletions(-)
diff --git a/install/ui/details.js b/install/ui/details.js
index 125ef1bc85bbdd5dea488061563539db41557dda..5872e81cff4aa97e0d35cd78d21e9fec75e617a5 100644
--- a/install/ui/details.js
+++ b/install/ui/details.js
@@ -333,6 +333,12 @@ IPA.details_section = function (spec){
return that;
};
+ that.multivalued_text = function(spec) {
+ var field = IPA.multivalued_text_widget(spec);
+ that.add_field(field);
+ return that;
+ };
+
that.create_field = function(spec) {
//TODO: replace IPA.details_field with class-specific implementation
@@ -510,17 +516,14 @@ IPA.details_list_section = function (spec){
for (var i = 0; i < fields.length; ++i) {
var field = fields[i];
- var label = field.label;
+ var label = field.label || '';
// no need to get i18n label from metadata
// because it's already done by field.init()
- if (label !== '') {
- label += ':';
- }
-
$('<dt/>', {
- html: label
+ html: label+':',
+ title: label
}).appendTo(dl);
var span = $('<span/>', { 'name': field.name }).appendTo(dl);
diff --git a/install/ui/ipa.css b/install/ui/ipa.css
index 1f34b3ce4ff98d1e92b01cec91902bba04ce5dc4..c851a400e65677963b2f3972fb5bd45642529459 100644
--- a/install/ui/ipa.css
+++ b/install/ui/ipa.css
@@ -246,7 +246,7 @@ dl.entryattrs dd {
dl.entryattrs dd.first {
margin-left: 0;
margin-top: 0.5em;
- font-weight: bold;
+ font-weight: bold;
}
dl.entryattrs dd.other {
@@ -256,7 +256,7 @@ dl.entryattrs dd.other {
dl.entryattrs input {
margin-right: 0.5em;
- margin-top: -1.2em;
+ margin-bottom: 1em;
}
dl.entryattrs input.otp {
diff --git a/install/ui/test/details_tests.js b/install/ui/test/details_tests.js
index fb33808c7faf60bbeda9090ebd20fa5a64b94113..cf4de0825cff3ef18eb6b421455ae5d650bb6f9c 100644
--- a/install/ui/test/details_tests.js
+++ b/install/ui/test/details_tests.js
@@ -21,12 +21,24 @@
module('details', {
setup: function() {
+ IPA.ajax_options.async = false;
+
+ IPA.init(
+ "data",
+ true,
+ function(data, text_status, xhr) {
+ },
+ function(xhr, text_status, error_thrown) {
+ ok(false, "ipa_init() failed: "+error_thrown);
+ }
+ );
+
var obj_name = 'user';
IPA.entity_factories.user=
function(){
return IPA.entity({name:obj_name});
};
- IPA.start_entities();
+ IPA.start_entities();
},
teardown: function() {
}
@@ -35,24 +47,13 @@ module('details', {
test("Testing IPA.details_section.create().", function() {
- IPA.ajax_options.async = false;
-
- IPA.init(
- "data",
- true,
- function(data, text_status, xhr) {
- ok(true, "ipa_init() succeeded.");
- },
- function(xhr, text_status, error_thrown) {
- ok(false, "ipa_init() failed: "+error_thrown);
- }
- );
-
var section = IPA.stanza({name:'IDIDID', label:'NAMENAMENAME'}).
input({name:'cn'}).
- input({name:'description'}).
- input({name:'number'});
+ input({name:'uid'}).
+ input({name:'mail'});
+ section.entity_name = 'user';
+ section.init();
var fields = section.fields;
var container = $("<div/>");
@@ -105,19 +106,6 @@ test("Testing IPA.details_section.create().", function() {
test("Testing details lifecycle: create, setup, load.", function(){
- IPA.ajax_options.async = false;
-
- IPA.init(
- "data",
- true,
- function(data, text_status, xhr) {
- ok(true, "ipa_init() succeeded.");
- },
- function(xhr, text_status, error_thrown) {
- ok(false, "ipa_init() failed: "+error_thrown);
- }
- );
-
var result = {};
IPA.cmd(
diff --git a/install/ui/test/widget_tests.js b/install/ui/test/widget_tests.js
index 3d827cdb88b2324ccaae4ab01dc2b021a12a56f0..f4281e38fa173de50fe5390c0f476781a2868377 100644
--- a/install/ui/test/widget_tests.js
+++ b/install/ui/test/widget_tests.js
@@ -67,7 +67,7 @@ function base_widget_test(widget,entity_name, value){
}
-function widget_string_test(widget, value){
+function widget_string_test(widget) {
var value = 'test_title';
var mock_record = {'title': value};
@@ -117,6 +117,31 @@ function text_tests(widget,input){
}
+function multivalued_text_tests(widget) {
+
+ var values = ['val1', 'val2', 'val3'];
+
+ var record = {};
+ record[widget.name] = values;
+
+ widget.load(record);
+
+ same(widget.save(), values, "All values loaded");
+ same(widget.is_dirty(), false, "Field initially clean");
+
+ values = ['val1', 'val2', 'val3', 'val4'];
+ widget.add_row('val4');
+
+ same(widget.save(), values, "Value added");
+ same(widget.is_dirty(), true, "Field is dirty");
+
+ values = ['val1', 'val3', 'val4'];
+ widget.remove_row(1);
+
+ same(widget.save(), values, "Value removed");
+ same(widget.is_dirty(), true, "Field is dirty");
+}
+
test("IPA.table_widget" ,function(){
var widget = IPA.table_widget({undo:true,name:'users'});
@@ -191,6 +216,13 @@ test("Testing text widget.", function() {
});
+test("Testing multi-valued text widget.", function() {
+ var widget = IPA.multivalued_text_widget({undo:true,name:'title'});
+ base_widget_test(widget,'user','test_value');
+ widget_string_test(widget);
+ multivalued_text_tests(widget);
+});
+
test("Testing checkbox widget.", function() {
var widget = IPA.checkbox_widget({name:'title'});
base_widget_test(widget,'user','test_value');
diff --git a/install/ui/user.js b/install/ui/user.js
index 1ce95009f466edce9a529efdc0dc255f3d296a45..663aab92092916433aca1a67636107754b38d931 100644
--- a/install/ui/user.js
+++ b/install/ui/user.js
@@ -64,11 +64,11 @@ IPA.entity_factories.user = function (){
input({name:'homedirectory'})).
section(
IPA.stanza({name: 'contact', label: IPA.messages.details.contact}).
- input({name:'mail'}).
- input({name:'telephonenumber'}).
- input({name:'pager'}).
- input({name:'mobile'}).
- input({name:'facsimiletelephonenumber'})).
+ multivalued_text({name:'mail'}).
+ multivalued_text({name:'telephonenumber'}).
+ multivalued_text({name:'pager'}).
+ multivalued_text({name:'mobile'}).
+ multivalued_text({name:'facsimiletelephonenumber'})).
section(
IPA.stanza({name: 'mailing', label: IPA.messages.details.mailing}).
input({name:'street'}).
diff --git a/install/ui/widget.js b/install/ui/widget.js
index 8f3eeb62f8dbf56d452b72890280852a77562fbd..db9d1744eb7c00979b92a8e10a3af8870e9e37bd 100644
--- a/install/ui/widget.js
+++ b/install/ui/widget.js
@@ -2,6 +2,7 @@
/* Authors:
* Endi Sukma Dewata <edew...@redhat.com>
* Adam Young <ayo...@redhat.com>
+ * Pavel Zuna <pz...@redhat.com>
*
* Copyright (C) 2010 Red Hat
* see file 'COPYING' for use and warranty information
@@ -34,8 +35,13 @@ IPA.widget = function(spec) {
that.tooltip = spec.tooltip;
that.disabled = spec.disabled;
+
+ // read_only is set during initialization
that.read_only = spec.read_only;
+ // writable is set during load
+ that.writable = true;
+
that._entity_name = spec.entity_name;
that.width = spec.width;
@@ -80,7 +86,7 @@ IPA.widget = function(spec) {
if ( !text || text.match(regex) ) {
error_link.css('display', 'none');
that.valid = true;
- }else{
+ } else {
error_link.css('display', 'block');
if (that.param_info.pattern_errmsg) {
error_link.html(that.param_info.pattern_errmsg);
@@ -127,6 +133,23 @@ IPA.widget = function(spec) {
that.values = value ? [value] : [];
}
+ that.writable = true;
+
+ if (that.param_info.primary_key) {
+ that.writable = false;
+ }
+
+ if ('no_update' in that.param_info.flags) {
+ that.writable = false;
+ }
+
+ if (that.record.attributelevelrights) {
+ var rights = that.record.attributelevelrights[that.name];
+ if (!rights || rights.indexOf('w') < 0) {
+ that.writable = false;
+ }
+ }
+
that.reset();
}
@@ -272,21 +295,21 @@ IPA.text_widget = function(spec) {
container.append(' ');
- $("<span/>",{
- name:'error_link',
- html:"Text does not match field pattern",
- "class":"ui-state-error ui-corner-all",
- style:"display:none"
+ $('<span/>', {
+ name: 'error_link',
+ html: 'Text does not match field pattern',
+ 'class': 'ui-state-error ui-corner-all',
+ style: 'display:none'
}).appendTo(container);
};
that.setup = function(container) {
- this.widget_setup(container);
+ that.widget_setup(container);
var input = $('input[name="'+that.name+'"]', that.container);
input.keyup(function() {
- if(that.undo){
+ if (that.undo) {
that.show_undo();
}
var value = $(this).val();
@@ -343,6 +366,269 @@ IPA.text_widget = function(spec) {
return that;
};
+IPA.multivalued_text_widget = function(spec) {
+
+ spec = spec || {};
+
+ var that = IPA.widget(spec);
+
+ that.size = spec.size || 30;
+
+ that.get_undo = function(index) {
+ if (index === undefined) {
+ return $('span[name="undo_all"]', that.container);
+
+ } else {
+ var row = that.get_row(index);
+ return $('span[name="undo"]', row);
+ }
+ };
+
+ that.show_undo = function(index) {
+ var undo = that.get_undo(index);
+ undo.css('display', 'inline');
+ };
+
+ that.hide_undo = function(index) {
+ var undo = that.get_undo(index);
+ undo.css('display', 'none');
+ };
+
+ that.create = function(container) {
+
+ var dd = $('<dd/>', {
+ 'class': 'first'
+ }).appendTo(container);
+
+ var div = $('<div/>', {
+ name: 'value'
+ }).appendTo(dd);
+
+ $('<input/>', {
+ type: 'text',
+ name: that.name,
+ disabled: that.disabled,
+ size: that.size,
+ title: that.tooltip
+ }).appendTo(div);
+
+ div.append(' ');
+
+ $('<a/>', {
+ name: 'remove',
+ href: 'jslink',
+ title: 'Remove',
+ html: 'Remove'
+ }).appendTo(div);
+
+ if (that.undo) {
+ div.append(' ');
+ that.create_undo(div);
+ }
+
+ div.append(' ');
+
+ $('<span/>', {
+ name: 'error_link',
+ html: 'Text does not match field pattern',
+ 'class': 'ui-state-error ui-corner-all',
+ style: 'display:none'
+ }).appendTo(div);
+
+ $('<a/>', {
+ name: 'add',
+ href: 'jslink',
+ title: 'Add',
+ html: 'Add'
+ }).appendTo(dd);
+
+ dd.append(' ');
+
+ $('<span/>', {
+ name: 'undo_all',
+ style: 'display: none;',
+ 'class': 'ui-state-highlight ui-corner-all undo',
+ html: 'undo all'
+ }).appendTo(dd);
+ };
+
+ that.setup = function(container) {
+
+ that.widget_setup(container);
+
+ that.template = $('div[name=value]', that.container);
+ that.template.detach();
+
+ var undo = that.get_undo();
+ undo.click(function() {
+ that.reset();
+ });
+
+ var add_link = $('a[name=add]', that.container);
+ add_link.click(function() {
+ that.add_row('');
+ var input = $('input[name="'+that.name+'"]:last', that.container);
+ input.focus();
+ return false;
+ });
+ };
+
+ that.save = function() {
+ var values = [];
+ if (that.read_only || !that.writable) {
+ $('label[name="'+that.name+'"]', that.container).each(function() {
+ var input = $(this);
+ var value = $.trim(input.html());
+ values.push(value);
+ });
+
+ } else {
+ $('input[name="'+that.name+'"]', that.container).each(function() {
+ var input = $(this);
+ if (input.is('.strikethrough')) return;
+
+ var value = $.trim(input.val());
+ values.push(value);
+ });
+ }
+ return values;
+ };
+
+ that.add_row = function(value) {
+
+ var add_link = $('a[name=add]', that.container);
+
+ var row = that.template.clone();
+ row.insertBefore(add_link);
+
+ var input = $('input[name="'+that.name+'"]', row);
+ var remove_link = $('a[name=remove]', row);
+ var undo_link = $('span[name=undo]', row);
+
+ if (that.read_only || !that.writable) {
+ var label = $('<label/>', {
+ 'name': that.name,
+ 'html': value
+ });
+ input.replaceWith(label);
+
+ remove_link.css('display', 'none');
+
+ } else {
+ input.val(value);
+
+ var index = that.row_index(row);
+ if (index >= that.values.length) {
+ // show undo/remove link for new value
+ if (that.undo) {
+ that.show_undo(index);
+ that.show_undo();
+ remove_link.css('display', 'none');
+ } else {
+ remove_link.css('display', 'inline');
+ }
+ }
+
+ input.keyup(function() {
+ var index = that.row_index(row);
+ // uncross removed value
+ input.removeClass('strikethrough');
+ if (that.undo) {
+ that.show_undo(index);
+ that.show_undo();
+ if (index < that.values.length) {
+ remove_link.css('display', 'inline');
+ }
+ }
+ var value = $(this).val();
+ that.validate_input(value);
+ });
+
+ remove_link.click(function() {
+ var index = that.row_index(row);
+ if (index < that.values.length) {
+ // restore old value then cross it out
+ that.update(index);
+ input.addClass('strikethrough');
+ if (that.undo) {
+ that.show_undo(index);
+ that.show_undo();
+ }
+ remove_link.css('display', 'none');
+ } else {
+ // remove new value
+ row.remove();
+ }
+ return false;
+ });
+
+ undo_link.click(function() {
+ var index = that.row_index(row);
+ if (index < that.values.length) {
+ // restore old value
+ that.reset(index);
+ input.removeClass('strikethrough');
+ remove_link.css('display', 'inline');
+ } else {
+ // remove new value
+ row.remove();
+ }
+ });
+ }
+ };
+
+ that.remove_row = function(index) {
+ that.get_row(index).remove();
+ };
+
+ that.get_row = function(index) {
+ return $('div[name=value]:eq('+index+')', that.container);
+ };
+
+ that.get_rows = function() {
+ return $('div[name=value]', that.container);
+ };
+
+ that.row_index = function(row) {
+ return that.get_rows().index(row);
+ };
+
+ that.reset = function(index) {
+ that.hide_undo(index);
+ that.update(index);
+ };
+
+ that.update = function(index) {
+
+ var value;
+
+ if (index === undefined) {
+ $('div[name=value]', that.container).remove();
+
+ for (var i=0; i<that.values.length; i++) {
+ value = that.values[i];
+ that.add_row(value);
+ }
+
+ var add_link = $('a[name=add]', that.container);
+
+ if (that.read_only || !that.writable) {
+ add_link.css('display', 'none');
+ } else {
+ add_link.css('display', 'inline');
+ }
+
+ } else {
+ value = that.values[index];
+ var row = that.get_row(index);
+ var input = $('input[name="'+that.name+'"]', row);
+ input.val(value);
+ }
+ };
+
+ return that;
+};
+
IPA.checkbox_widget = function (spec) {
spec = spec || {};
--
1.6.6.1
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
