Re: [Freeipa-devel] [PATCH] admiyo-0191-target-section-without-radio-buttons
On 02/10/2011 06:46 PM, Endi Sukma Dewata wrote:
On 2/10/2011 5:02 PM, Adam Young wrote:
On 02/10/2011 04:42 PM, Endi Sukma Dewata wrote:
On 2/10/2011 3:27 PM, Adam Young wrote:
NACK. As discussed over IRC, the is_dirty functionality is not
working for permissions that have an object by type target.
Was worse than that, load was broken.
It still has some problems:
1. Updating a permission with a filter doesn't work. Clicking the
update button didn't execute anything, the undo button didn't disappear.
2. Resetting the user details page is not working properly, some
fields did not get reset. I think the addition of undo_span in
widgets.js is not needed and causing a problem because not all
(custom) widgets will call create_undo().
Filter not set was due to incomplete filter_text attribute-rights work
around
undo_span is now wrapped in jquery select, so that it there is no undo,
it works correctly.
From 70a7496c6026a2ebe77535ac5abb84dae26303ab Mon Sep 17 00:00:00 2001
From: Adam Young ayo...@redhat.com
Date: Mon, 7 Feb 2011 23:02:43 -0500
Subject: [PATCH] target section without radio buttons
ACI target section refactored into an array of widget-like objects.
The radio buttons have been replaced by a select box. THe select is not visible on the details page.
---
install/ui/aci.js| 467 +-
install/ui/dialog.js |7 +
install/ui/test/aci_tests.js | 43 +++-
install/ui/widget.js |7 +-
4 files changed, 277 insertions(+), 247 deletions(-)
diff --git a/install/ui/aci.js b/install/ui/aci.js
index e515902c5c83451389b5c9dde8115e087f9686f3..db1267936bf4f6a12ae8eadc278f27fc075b6f51 100644
--- a/install/ui/aci.js
+++ b/install/ui/aci.js
@@ -36,6 +36,8 @@ IPA.attributes_widget = function(spec) {
that.create = function(container) {
+that.container = container;
+
that.table = $('table/', {
id:id,
'class':'search-table aci-attribute-table'
@@ -200,6 +202,9 @@ IPA.hidden_widget = function(spec) {
that.reset = function(){
};
+that.is_dirty = function(){
+return false;
+}
return that;
};
@@ -221,265 +226,263 @@ IPA.target_section = function(spec) {
spec = spec || {};
var that = IPA.details_section(spec);
-
that.undo = typeof spec.undo == 'undefined' ? true : spec.undo;
-var groupings = ['aci_by_type', 'aci_by_query', 'aci_by_group',
- 'aci_by_filter' ];
-var inputs = ['input', 'select', 'textarea'];
-
-function disable_inputs() {
-for (var g = 0; g groupings.length; g += 1 ){
-for (var t = 0 ; t inputs.length; t += 1){
-$('.' + groupings[g] + ' '+ inputs[t]).
-attr('disabled', 'disabled');
+that.filter_text = IPA.text_widget({name: 'filter', undo: that.undo});
+that.subtree_textarea = IPA.textarea_widget({
+name: 'subtree',
+cols: 30, rows: 1,
+undo: that.undo
+});
+that.group_select = IPA.entity_select_widget(
+{name: 'targetgroup', entity:'group', undo: that.undo});
+that.type_select = IPA.select_widget({name: 'type', undo: that.undo});
+that.attribute_table = IPA.attributes_widget({name: 'attrs', undo: that.undo});
+//TODO make the add_dialog.save not require fields, just use the record
+that.add_field(that.filter_text);
+that.add_field(that.subtree_textarea);
+that.add_field(that.group_select );
+that.add_field(that.type_select);
+that.add_field(that.attribute_table);
+
+var target_types = [
+{
+name:'filter',
+create: function(dl){
+
+$('dt/').
+append($('label/', {
+text: 'Filter'
+})).
+appendTo(dl);
+
+var dd = $('dd/', {
+'class': 'aci_by_filter first'
+}).appendTo(dl);
+
+var span = $('span/', {
+name: 'filter'
+}).appendTo(dd);
+
+that.filter_text.create(span);
+},
+load: function(record){
+that.filter_text.load(record);
+},
+save: function(record){
+record.filter = that.filter_text.save()[0];
+}
+},
+{
+name:'subtree',
+create:function(dl) {
+ $('dt/').
+ append($('label/', {
+ text: 'By Subtree'
+ })).
+ appendTo(dl);
+ var dd = $('dd/', {
+ 'class': 'aci_by_query first'
+ }).appendTo(dl);
+ var span = $('span/', {
+ name: 'subtree'
+ }).appendTo(dd);
+ that.subtree_textarea.create(span);
+},
+load: function(record){
+
Re: [Freeipa-devel] [PATCH] admiyo-0191-target-section-without-radio-buttons
On 2/11/2011 11:52 AM, Adam Young wrote: Filter not set was due to incomplete filter_text attribute-rights work around undo_span is now wrapped in jquery select, so that it there is no undo, it works correctly. ACK. The filter problem is fixed in 192. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] admiyo-0191-target-section-without-radio-buttons
On 02/11/2011 02:28 PM, Endi Sukma Dewata wrote: On 2/11/2011 11:52 AM, Adam Young wrote: Filter not set was due to incomplete filter_text attribute-rights work around undo_span is now wrapped in jquery select, so that it there is no undo, it works correctly. ACK. The filter problem is fixed in 192. Pushed to master ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] admiyo-0191-target-section-without-radio-buttons
On 02/10/2011 01:13 AM, Endi Sukma Dewata wrote:
On 2/9/2011 7:06 PM, Adam Young wrote:
A few comments:
1. The functionality seems to be working, but the layout is a bit
different. Previously the label (e.g. Filter) and the widget (e.g.
text field) occupy the same line. Right now they occupy different
lines and not aligned with the labels widgets above it (e.g.
Permission name). I'd like the UXD team to review this change.
I had mIssed the classes that these things needed. Added them back in.
2. The jQuery selectors on lines 427, 462, 472 in aci.js are not
qualified, so they will be doing a global search. I'd rather store the
object reference somewhere and use it directly without searching for
it again. For example, line 411 can be changed as follows:
target_type.container = $('dl/', {
Then line 427 can be changed as follows:
target_type.container.css('display', 'block');
Done. Good idea/
3. The indentation of the target_types array in aci.js is inconsistent.
Fixed
4. The IPA.hidden_widget doesn't seem to be used. Should this be removed?
Gone baby gone
5. For the changes in dialog.js, it's not necessary to check
section.reset()'s presence before calling it. All sections will have a
reset() function because it's inherited from the base class.
Removed
6. For the changes in widget.js, let's do this in a separate patch.
We'll combine the create/setup in a more consistent way.
Agreed. This was actually part of trial and error to get it to work,
and it didn't need to be there. Gone.
7. There are some jslint warnings.
Fixed
From c88f50789f8ae94e852b15aaf8970f5c506554f9 Mon Sep 17 00:00:00 2001
From: Adam Young ayo...@redhat.com
Date: Mon, 7 Feb 2011 23:02:43 -0500
Subject: [PATCH] target section without radio buttons
ACI target section refactored into an array of widget-like objects.
The radio buttons have been replaced by a select box. THe select is not visible on the details page.
https://fedorahosted.org/freeipa/ticket/924
---
install/ui/aci.js| 519 +-
install/ui/dialog.js |3 +
install/ui/test/aci_tests.js | 43 +++-
install/ui/widget.js |6 +-
4 files changed, 293 insertions(+), 278 deletions(-)
diff --git a/install/ui/aci.js b/install/ui/aci.js
index e515902c5c83451389b5c9dde8115e087f9686f3..fce6846dc56ec4722239673f6b9fc9ec2c939aa9 100644
--- a/install/ui/aci.js
+++ b/install/ui/aci.js
@@ -36,6 +36,8 @@ IPA.attributes_widget = function(spec) {
that.create = function(container) {
+
+
that.table = $('table/', {
id:id,
'class':'search-table aci-attribute-table'
@@ -129,7 +131,8 @@ IPA.attributes_widget = function(spec) {
var unmatched = [];
for (var i=0; ithat.values.length; i++) {
-var input = $('input[name='+that.name+'][value='+that.values[i]+']', that.container);
+var input = $('input[name='+that.name+']'+
+ '[value='+that.values[i]+']', that.container);
if (!input.length) {
unmatched.push(that.values[i]);
}
@@ -180,29 +183,6 @@ IPA.rights_widget = function(spec) {
return that;
};
-IPA.hidden_widget = function(spec) {
-spec.label = '';
-var that = IPA.widget(spec);
-that.id = spec.id;
-var value = spec.value || '';
-that.create = function(container){
-$('input/',{
-type:'hidden',
-'id':that.id,
-value: value
-}).
-appendTo(container);
-};
-
-that.save = function(){
-return [value];
-};
-that.reset = function(){
-
-};
-return that;
-};
-
IPA.rights_section = function() {
var spec = {
@@ -210,7 +190,8 @@ IPA.rights_section = function() {
'label': 'Rights'
};
var that = IPA.details_section(spec);
-that.add_field(IPA.rights_widget({name: 'permissions', label: 'Permissions', join: true}));
+that.add_field(IPA.rights_widget(
+{name: 'permissions', label: 'Permissions', join: true}));
return that;
};
@@ -221,265 +202,268 @@ IPA.target_section = function(spec) {
spec = spec || {};
var that = IPA.details_section(spec);
-
that.undo = typeof spec.undo == 'undefined' ? true : spec.undo;
-var groupings = ['aci_by_type', 'aci_by_query', 'aci_by_group',
- 'aci_by_filter' ];
-var inputs = ['input', 'select', 'textarea'];
-
-function disable_inputs() {
-for (var g = 0; g groupings.length; g += 1 ){
-for (var t = 0 ; t inputs.length; t += 1){
-$('.' + groupings[g] + ' '+ inputs[t]).
-attr('disabled', 'disabled');
+that.filter_text = IPA.text_widget({name: 'filter', undo: that.undo});
+that.subtree_textarea = IPA.textarea_widget({
+name: 'subtree',
+cols: 30, rows: 1,
+undo: that.undo
+});
+that.group_select =
Re: [Freeipa-devel] [PATCH] admiyo-0191-target-section-without-radio-buttons
Last version was a little too zealos in remivng style info, and I
removed the code that hid the select boxthat chose the target. Added
that code back in here.
On 02/10/2011 03:02 PM, Adam Young wrote:
On 02/10/2011 01:13 AM, Endi Sukma Dewata wrote:
On 2/9/2011 7:06 PM, Adam Young wrote:
A few comments:
1. The functionality seems to be working, but the layout is a bit
different. Previously the label (e.g. Filter) and the widget (e.g.
text field) occupy the same line. Right now they occupy different
lines and not aligned with the labels widgets above it (e.g.
Permission name). I'd like the UXD team to review this change.
I had mIssed the classes that these things needed. Added them back in.
2. The jQuery selectors on lines 427, 462, 472 in aci.js are not
qualified, so they will be doing a global search. I'd rather store
the object reference somewhere and use it directly without searching
for it again. For example, line 411 can be changed as follows:
target_type.container = $('dl/', {
Then line 427 can be changed as follows:
target_type.container.css('display', 'block');
Done. Good idea/
3. The indentation of the target_types array in aci.js is inconsistent.
Fixed
4. The IPA.hidden_widget doesn't seem to be used. Should this be
removed?
Gone baby gone
5. For the changes in dialog.js, it's not necessary to check
section.reset()'s presence before calling it. All sections will have
a reset() function because it's inherited from the base class.
Removed
6. For the changes in widget.js, let's do this in a separate patch.
We'll combine the create/setup in a more consistent way.
Agreed. This was actually part of trial and error to get it to work,
and it didn't need to be there. Gone.
7. There are some jslint warnings.
Fixed
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
From 6c157b6e49b2cf5da7f848d6bf4c40f92ad7b7b4 Mon Sep 17 00:00:00 2001
From: Adam Young ayo...@redhat.com
Date: Mon, 7 Feb 2011 23:02:43 -0500
Subject: [PATCH] target section without radio buttons
ACI target section refactored into an array of widget-like objects.
The radio buttons have been replaced by a select box. THe select is not visible on the details page.
https://fedorahosted.org/freeipa/ticket/924
---
install/ui/aci.js| 519 +-
install/ui/dialog.js |3 +
install/ui/test/aci_tests.js | 43 +++-
install/ui/widget.js |6 +-
4 files changed, 293 insertions(+), 278 deletions(-)
diff --git a/install/ui/aci.js b/install/ui/aci.js
index e515902c5c83451389b5c9dde8115e087f9686f3..9307785b8c5d3850e53943356d1198ed66c417aa 100644
--- a/install/ui/aci.js
+++ b/install/ui/aci.js
@@ -36,6 +36,8 @@ IPA.attributes_widget = function(spec) {
that.create = function(container) {
+
+
that.table = $('table/', {
id:id,
'class':'search-table aci-attribute-table'
@@ -129,7 +131,8 @@ IPA.attributes_widget = function(spec) {
var unmatched = [];
for (var i=0; ithat.values.length; i++) {
-var input = $('input[name='+that.name+'][value='+that.values[i]+']', that.container);
+var input = $('input[name='+that.name+']'+
+ '[value='+that.values[i]+']', that.container);
if (!input.length) {
unmatched.push(that.values[i]);
}
@@ -180,29 +183,6 @@ IPA.rights_widget = function(spec) {
return that;
};
-IPA.hidden_widget = function(spec) {
-spec.label = '';
-var that = IPA.widget(spec);
-that.id = spec.id;
-var value = spec.value || '';
-that.create = function(container){
-$('input/',{
-type:'hidden',
-'id':that.id,
-value: value
-}).
-appendTo(container);
-};
-
-that.save = function(){
-return [value];
-};
-that.reset = function(){
-
-};
-return that;
-};
-
IPA.rights_section = function() {
var spec = {
@@ -210,7 +190,8 @@ IPA.rights_section = function() {
'label': 'Rights'
};
var that = IPA.details_section(spec);
-that.add_field(IPA.rights_widget({name: 'permissions', label: 'Permissions', join: true}));
+that.add_field(IPA.rights_widget(
+{name: 'permissions', label: 'Permissions', join: true}));
return that;
};
@@ -221,265 +202,268 @@ IPA.target_section = function(spec) {
spec = spec || {};
var that = IPA.details_section(spec);
-
that.undo = typeof spec.undo == 'undefined' ? true : spec.undo;
-var groupings = ['aci_by_type', 'aci_by_query', 'aci_by_group',
- 'aci_by_filter' ];
-var inputs = ['input', 'select', 'textarea'];
-
-function disable_inputs() {
-for (var g = 0; g groupings.length; g += 1 ){
-for (var t = 0 ; t
Re: [Freeipa-devel] [PATCH] admiyo-0191-target-section-without-radio-buttons
On 02/10/2011 03:09 PM, Adam Young wrote:
Last version was a little too zealos in remivng style info, and I
removed the code that hid the select boxthat chose the target. Added
that code back in here.
On 02/10/2011 03:02 PM, Adam Young wrote:
On 02/10/2011 01:13 AM, Endi Sukma Dewata wrote:
On 2/9/2011 7:06 PM, Adam Young wrote:
A few comments:
1. The functionality seems to be working, but the layout is a bit
different. Previously the label (e.g. Filter) and the widget (e.g.
text field) occupy the same line. Right now they occupy different
lines and not aligned with the labels widgets above it (e.g.
Permission name). I'd like the UXD team to review this change.
I had mIssed the classes that these things needed. Added them back in.
2. The jQuery selectors on lines 427, 462, 472 in aci.js are not
qualified, so they will be doing a global search. I'd rather store
the object reference somewhere and use it directly without searching
for it again. For example, line 411 can be changed as follows:
target_type.container = $('dl/', {
Then line 427 can be changed as follows:
target_type.container.css('display', 'block');
Done. Good idea/
3. The indentation of the target_types array in aci.js is inconsistent.
Fixed
4. The IPA.hidden_widget doesn't seem to be used. Should this be
removed?
Gone baby gone
5. For the changes in dialog.js, it's not necessary to check
section.reset()'s presence before calling it. All sections will have
a reset() function because it's inherited from the base class.
Removed
6. For the changes in widget.js, let's do this in a separate patch.
We'll combine the create/setup in a more consistent way.
Agreed. This was actually part of trial and error to get it to work,
and it didn't need to be there. Gone.
7. There are some jslint warnings.
Fixed
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
From ea289c7bbcb58b86b1c9fd61f8c03e360e476e03 Mon Sep 17 00:00:00 2001
From: Adam Young ayo...@redhat.com
Date: Mon, 7 Feb 2011 23:02:43 -0500
Subject: [PATCH] target section without radio buttons
ACI target section refactored into an array of widget-like objects.
The radio buttons have been replaced by a select box. THe select is not visible on the details page.
https://fedorahosted.org/freeipa/ticket/924
---
install/ui/aci.js| 519 +-
install/ui/dialog.js |3 +
install/ui/test/aci_tests.js | 43 +++-
install/ui/widget.js |3 +-
4 files changed, 293 insertions(+), 275 deletions(-)
diff --git a/install/ui/aci.js b/install/ui/aci.js
index e515902c5c83451389b5c9dde8115e087f9686f3..182cd7e05db230dde9bbb63aa8c21e877dfb8a43 100644
--- a/install/ui/aci.js
+++ b/install/ui/aci.js
@@ -36,6 +36,8 @@ IPA.attributes_widget = function(spec) {
that.create = function(container) {
+
+
that.table = $('table/', {
id:id,
'class':'search-table aci-attribute-table'
@@ -129,7 +131,8 @@ IPA.attributes_widget = function(spec) {
var unmatched = [];
for (var i=0; ithat.values.length; i++) {
-var input = $('input[name='+that.name+'][value='+that.values[i]+']', that.container);
+var input = $('input[name='+that.name+']'+
+ '[value='+that.values[i]+']', that.container);
if (!input.length) {
unmatched.push(that.values[i]);
}
@@ -180,29 +183,6 @@ IPA.rights_widget = function(spec) {
return that;
};
-IPA.hidden_widget = function(spec) {
-spec.label = '';
-var that = IPA.widget(spec);
-that.id = spec.id;
-var value = spec.value || '';
-that.create = function(container){
-$('input/',{
-type:'hidden',
-'id':that.id,
-value: value
-}).
-appendTo(container);
-};
-
-that.save = function(){
-return [value];
-};
-that.reset = function(){
-
-};
-return that;
-};
-
IPA.rights_section = function() {
var spec = {
@@ -210,7 +190,8 @@ IPA.rights_section = function() {
'label': 'Rights'
};
var that = IPA.details_section(spec);
-that.add_field(IPA.rights_widget({name: 'permissions', label: 'Permissions', join: true}));
+that.add_field(IPA.rights_widget(
+{name: 'permissions', label: 'Permissions', join: true}));
return that;
};
@@ -221,265 +202,270 @@ IPA.target_section = function(spec) {
spec = spec || {};
var that = IPA.details_section(spec);
-
that.undo = typeof spec.undo == 'undefined' ? true : spec.undo;
-var groupings = ['aci_by_type', 'aci_by_query', 'aci_by_group',
-
Re: [Freeipa-devel] [PATCH] admiyo-0191-target-section-without-radio-buttons
On 2/10/2011 3:27 PM, Adam Young wrote: NACK. As discussed over IRC, the is_dirty functionality is not working for permissions that have an object by type target. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] admiyo-0191-target-section-without-radio-buttons
On 2/10/2011 5:02 PM, Adam Young wrote: On 02/10/2011 04:42 PM, Endi Sukma Dewata wrote: On 2/10/2011 3:27 PM, Adam Young wrote: NACK. As discussed over IRC, the is_dirty functionality is not working for permissions that have an object by type target. Was worse than that, load was broken. It still has some problems: 1. Updating a permission with a filter doesn't work. Clicking the update button didn't execute anything, the undo button didn't disappear. 2. Resetting the user details page is not working properly, some fields did not get reset. I think the addition of undo_span in widgets.js is not needed and causing a problem because not all (custom) widgets will call create_undo(). -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] admiyo-0191-target-section-without-radio-buttons
On 2/9/2011 7:06 PM, Adam Young wrote:
A few comments:
1. The functionality seems to be working, but the layout is a bit
different. Previously the label (e.g. Filter) and the widget (e.g. text
field) occupy the same line. Right now they occupy different lines and
not aligned with the labels widgets above it (e.g. Permission name).
I'd like the UXD team to review this change.
2. The jQuery selectors on lines 427, 462, 472 in aci.js are not
qualified, so they will be doing a global search. I'd rather store the
object reference somewhere and use it directly without searching for it
again. For example, line 411 can be changed as follows:
target_type.container = $('dl/', {
Then line 427 can be changed as follows:
target_type.container.css('display', 'block');
3. The indentation of the target_types array in aci.js is inconsistent.
4. The IPA.hidden_widget doesn't seem to be used. Should this be removed?
5. For the changes in dialog.js, it's not necessary to check
section.reset()'s presence before calling it. All sections will have a
reset() function because it's inherited from the base class.
6. For the changes in widget.js, let's do this in a separate patch.
We'll combine the create/setup in a more consistent way.
7. There are some jslint warnings.
--
Endi S. Dewata
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
