Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE
On 02/08/2013 02:27 PM, Petr Vobornik wrote: Checkbox for NONE option was added. https://fedorahosted.org/freeipa/ticket/3404 Patches for master and 3.1 branch attached. I pushed this version to ipa-3-1 branch only as a hotfix for upcoming 3.1 release so that users can set NONE option via UI. The right solution should be chosen and pushed to master branch. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE
On 3/7/2013 7:37 AM, Petr Vobornik wrote: Ideally it should be generic enough to combine any widgets. This might be a common scenario somewhere else: Something: ( ) Option 1 ( ) Option 2 (o) Other: [something else ] This design has a flaw: https://fedorahosted.org/freeipa/ticket/3404#comment:5 I think this one makes the most sense to me: PAC Types: ( ) Inherited setting: ... inherited values ... (o) Override inherited setting [ ] MS-PAC [ ] PAD It looks like the NONE option is identical to not using the inherited values but also not selecting any new values, so we don't actually need a separate radio button for NONE because it can be represented by the above UI without redundancy. We just need better labels to explain the radio buttons. Maybe someone can come up with better labels than these. I implemented following design: https://fedorahosted.org/freeipa/ticket/3404#comment:7 It works but I can't imagine how it would look if we have more than two PAC types. I don't think we want to list every possible combinations. The above design is more future proof. Patch attached (255-1). I have a dilemma. I practically implemented the previous design (and then I've found the flaw..). Patches attached as wip-fre... I wonder if we can use it somehow or we should ditch it. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE
On 02/14/2013 04:56 PM, Endi Sukma Dewata wrote: On 2/14/2013 6:30 AM, Petr Vobornik wrote: If they are mutually exclusive, they probably should be separated using radio buttons like this: PAC: ( ) None (o) Type: [x] MS-PAC [ ] PAD You missed one option: nothing selected. It can be solved by adding '( ) Inherited' radio. I wouldn't have guessed that :) I agree we should add the 'Inherited' option. Anyway, this design seems more user friendly for more general audience than mine so I will implement it. The only problem with it is that one have to come with new label for each group and empty value - can't be inferred from metadata. Is there any issue adding new labels at this point? Worst case we could hard code the label now and add a translation later. It might be better to use a composite widget of radio buttons and checkboxes so we can reuse the code. Probably the definition will look something like this: { name: 'ipakrbauthzdata', type: 'radio', Not sure if it should be radio, more like something new. Right, probably the current radio widget can't do this. So either we improve the radio widget or create something new. label: ..., options: [ { label: ..., value: 'NONE' }, { label: ..., type: 'checkboxes', Do you expect to be there something different than checkboxes, or do you want it to do it this way for possible future customization. Ideally it should be generic enough to combine any widgets. This might be a common scenario somewhere else: Something: ( ) Option 1 ( ) Option 2 (o) Other: [something else ] This design has a flaw: https://fedorahosted.org/freeipa/ticket/3404#comment:5 I implemented following design: https://fedorahosted.org/freeipa/ticket/3404#comment:7 Patch attached (255-1). I have a dilemma. I practically implemented the previous design (and then I've found the flaw..). Patches attached as wip-fre... I wonder if we can use it somehow or we should ditch it. -- Petr Vobornik From 854fb91a1fdc45f4d1ea45b9dc18f82b52a83e6d Mon Sep 17 00:00:00 2001 From: Petr Vobornik pvobo...@redhat.com Date: Thu, 7 Mar 2013 14:16:21 +0100 Subject: [PATCH] Added Web UI support for service PAC type option: NONE ipakrbauthzdata accepts [null, 'NONE', 'MS-PAC, 'PAD']. 'MS-PAC' and 'PAD' can be combined together but they are mutually exclusive with 'NONE'. Hence, we can't use normal radio buttons nor checkboxes. New multivalued radio widget was developed to solve the problem. One can define option with multiple values. Thus following layout is possible: PAC type: ( ) Inherited ( ) None (o) MS-PAC ( ) PAD ( ) MS-PAC + PAD https://fedorahosted.org/freeipa/ticket/3404 --- install/ui/src/freeipa/field.js| 1 + install/ui/src/freeipa/service.js | 20 +++-- install/ui/src/freeipa/widget.js | 44 ++ install/ui/test/data/ipa_init.json | 5 + ipalib/plugins/internal.py | 5 + 5 files changed, 73 insertions(+), 2 deletions(-) diff --git a/install/ui/src/freeipa/field.js b/install/ui/src/freeipa/field.js index f705ef7b899f502b717daa384c01763c96aea664..efa9568124c0ebc59fdfc93c55a2bb888a616f90 100644 --- a/install/ui/src/freeipa/field.js +++ b/install/ui/src/freeipa/field.js @@ -921,6 +921,7 @@ IPA.field_factories['entity_select'] = IPA.field; IPA.field_factories['field'] = IPA.field; IPA.field_factories['link'] = IPA.link_field; IPA.field_factories['multivalued'] = IPA.multivalued_field; +IPA.field_factories['multivalued_radio'] = IPA.radio_field; IPA.field_factories['password'] = IPA.field; IPA.field_factories['radio'] = IPA.radio_field; IPA.field_factories['select'] = IPA.select_field; diff --git a/install/ui/src/freeipa/service.js b/install/ui/src/freeipa/service.js index ecb8ce9b30518bd620dbd6ffab05342cff7a946b..8e21a9a225e2fcf5b34512c85bd471dd2cf71704 100644 --- a/install/ui/src/freeipa/service.js +++ b/install/ui/src/freeipa/service.js @@ -54,8 +54,24 @@ IPA.service.entity = function(spec) { }, { name: 'ipakrbauthzdata', -type: 'checkboxes', -options: IPA.create_options(['MS-PAC', 'PAD']) +type: 'multivalued_radio', +direction: 'vertical', +options: IPA.create_options([ +{ +label: IPA.messages.krbauthzdata.inherited, +value: '' +}, +{ +label: IPA.messages.krbauthzdata.none, +value: 'NONE' +}, +
Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE
On Thu, Feb 14, 2013 at 09:56:44AM -0600, Endi Sukma Dewata wrote: On 2/14/2013 6:30 AM, Petr Vobornik wrote: If they are mutually exclusive, they probably should be separated using radio buttons like this: PAC: ( ) None (o) Type: [x] MS-PAC [ ] PAD You missed one option: nothing selected. It can be solved by adding '( ) Inherited' radio. I wouldn't have guessed that :) I agree we should add the 'Inherited' option. Would it be possible to print the inherited values here, i.e. the default values given in the ipakrbauthzdata attribute of ipaGuiConfig object? I think this would improve the user experience because you do not have to remember/look up the default values. bye, Sumit ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE
On 02/19/2013 01:40 PM, Sumit Bose wrote: On Thu, Feb 14, 2013 at 09:56:44AM -0600, Endi Sukma Dewata wrote: On 2/14/2013 6:30 AM, Petr Vobornik wrote: If they are mutually exclusive, they probably should be separated using radio buttons like this: PAC: ( ) None (o) Type: [x] MS-PAC [ ] PAD You missed one option: nothing selected. It can be solved by adding '( ) Inherited' radio. I wouldn't have guessed that :) I agree we should add the 'Inherited' option. Would it be possible to print the inherited values here, i.e. the default values given in the ipakrbauthzdata attribute of ipaGuiConfig object? I think this would improve the user experience because you do not have to remember/look up the default values. bye, Sumit It is possible, but not straightforward. I see two options: 1) Currently Web UI loads config at start so we can use it. The disadvantage is that the value might be changed and so the displayed value might be incorrect. 2) Other option is to run config-show along with service-show in a batch to get the up-to-date value. I'm not a big fan of #2 but it is probably better solution. -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE
On Tue, Feb 19, 2013 at 02:01:24PM +0100, Petr Vobornik wrote: On 02/19/2013 01:40 PM, Sumit Bose wrote: On Thu, Feb 14, 2013 at 09:56:44AM -0600, Endi Sukma Dewata wrote: On 2/14/2013 6:30 AM, Petr Vobornik wrote: If they are mutually exclusive, they probably should be separated using radio buttons like this: PAC: ( ) None (o) Type: [x] MS-PAC [ ] PAD You missed one option: nothing selected. It can be solved by adding '( ) Inherited' radio. I wouldn't have guessed that :) I agree we should add the 'Inherited' option. Would it be possible to print the inherited values here, i.e. the default values given in the ipakrbauthzdata attribute of ipaGuiConfig object? I think this would improve the user experience because you do not have to remember/look up the default values. bye, Sumit It is possible, but not straightforward. I see two options: 1) Currently Web UI loads config at start so we can use it. The disadvantage is that the value might be changed and so the displayed value might be incorrect. 2) Other option is to run config-show along with service-show in a batch to get the up-to-date value. I'm not a big fan of #2 but it is probably better solution. I agree with both :-). Since it is not straightforward I guess it would be better not to add this with this ticket but open a new one. Do you agree? bye, Sumit -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE
On 02/19/2013 02:08 PM, Sumit Bose wrote: On Tue, Feb 19, 2013 at 02:01:24PM +0100, Petr Vobornik wrote: On 02/19/2013 01:40 PM, Sumit Bose wrote: On Thu, Feb 14, 2013 at 09:56:44AM -0600, Endi Sukma Dewata wrote: On 2/14/2013 6:30 AM, Petr Vobornik wrote: If they are mutually exclusive, they probably should be separated using radio buttons like this: PAC: ( ) None (o) Type: [x] MS-PAC [ ] PAD You missed one option: nothing selected. It can be solved by adding '( ) Inherited' radio. I wouldn't have guessed that :) I agree we should add the 'Inherited' option. Would it be possible to print the inherited values here, i.e. the default values given in the ipakrbauthzdata attribute of ipaGuiConfig object? I think this would improve the user experience because you do not have to remember/look up the default values. bye, Sumit It is possible, but not straightforward. I see two options: 1) Currently Web UI loads config at start so we can use it. The disadvantage is that the value might be changed and so the displayed value might be incorrect. 2) Other option is to run config-show along with service-show in a batch to get the up-to-date value. I'm not a big fan of #2 but it is probably better solution. I agree with both :-). Since it is not straightforward I guess it would be better not to add this with this ticket but open a new one. Do you agree? Yes bye, Sumit -- Petr Vobornik -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE
On Tue, Feb 19, 2013 at 02:12:24PM +0100, Petr Vobornik wrote: On 02/19/2013 02:08 PM, Sumit Bose wrote: On Tue, Feb 19, 2013 at 02:01:24PM +0100, Petr Vobornik wrote: On 02/19/2013 01:40 PM, Sumit Bose wrote: On Thu, Feb 14, 2013 at 09:56:44AM -0600, Endi Sukma Dewata wrote: On 2/14/2013 6:30 AM, Petr Vobornik wrote: If they are mutually exclusive, they probably should be separated using radio buttons like this: PAC: ( ) None (o) Type: [x] MS-PAC [ ] PAD You missed one option: nothing selected. It can be solved by adding '( ) Inherited' radio. I wouldn't have guessed that :) I agree we should add the 'Inherited' option. Would it be possible to print the inherited values here, i.e. the default values given in the ipakrbauthzdata attribute of ipaGuiConfig object? I think this would improve the user experience because you do not have to remember/look up the default values. bye, Sumit It is possible, but not straightforward. I see two options: 1) Currently Web UI loads config at start so we can use it. The disadvantage is that the value might be changed and so the displayed value might be incorrect. 2) Other option is to run config-show along with service-show in a batch to get the up-to-date value. I'm not a big fan of #2 but it is probably better solution. I agree with both :-). Since it is not straightforward I guess it would be better not to add this with this ticket but open a new one. Do you agree? Yes Thanks, I've opened https://fedorahosted.org/freeipa/ticket/3436 . bye, Sumit bye, Sumit -- Petr Vobornik -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE
On 02/12/2013 11:15 PM, Endi Sukma Dewata wrote: On 2/12/2013 10:56 AM, Petr Vobornik wrote: We were discussing to NACK this approach. The implementation should be improved because of the mutually exclusive nature of NONE option with [MS-PAC, PAD] options. I think we should add spec definition (to Web UI only, or into server plugin as well) of mutex options. Something like: mutex_groups: [[['NONE'],['MS-PAC', 'PAD']], [/*another array of groups*/]]; basically an array of group arrays where group array would contain arrays of mutually exclusive option. Is it over-complicated? Would one array or a pair of groups be enough? That way we can disable and uncheck(should not happen, but to keep data consistency) checkboxes of options from other groups when user selects a value of some group. If they are mutually exclusive, they probably should be separated using radio buttons like this: PAC: ( ) None (o) Type: [x] MS-PAC [ ] PAD You missed one option: nothing selected. It can be solved by adding '( ) Inherited' radio. Anyway, this design seems more user friendly for more general audience than mine so I will implement it. The only problem with it is that one have to come with new label for each group and empty value - can't be inferred from metadata. It might be better to use a composite widget of radio buttons and checkboxes so we can reuse the code. Probably the definition will look something like this: { name: 'ipakrbauthzdata', type: 'radio', Not sure if it should be radio, more like something new. label: ..., options: [ { label: ..., value: 'NONE' }, { label: ..., type: 'checkboxes', Do you expect to be there something different than checkboxes, or do you want it to do it this way for possible future customization. options: [ { label: ..., value: 'MS-PAC' }, { label: ..., value: 'PAD' } ] } ] } The composite widget will handle setting the appropriate widgets when the value of the field on load. It will handle enabling/disabling the checkboxes when the radio button is selected. It will also compute the final value of the field from selected radio button/checkboxes on save. Or just dim (no disable) and uncheck. That way there would still be visual distinction and one don't have to uncheck all the options from one group when he wants to select options of mutex group. That is also possible, but it changes the normal behavior of checkboxes, so probably users would have to play around with it to understand the grouping. They could also get confused, e.g. is dimmed checkbox disabled? As a separate issue, we might want to fetch the options from metadata, when we want to show the values and don't care about creating different labels. If we use radio buttons like above, new labels are necessary to describe the different groups of checkboxes. For all radio buttons checkboxes in general, if we don't want to use labels we probably could simplify the definition such that we can specify the string values directly (without nested object): { name: 'ipakrbauthzdata', type: 'radio', options: [ 'NONE', { type: 'checkboxes', options: ['MS-PAC', 'PAD'] } ] } +1 for classic checkboxes or radios. Problem with this example is that, it is missing label for the checkbox group which can't be inferred from anything. -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE
On 2/14/2013 6:30 AM, Petr Vobornik wrote: If they are mutually exclusive, they probably should be separated using radio buttons like this: PAC: ( ) None (o) Type: [x] MS-PAC [ ] PAD You missed one option: nothing selected. It can be solved by adding '( ) Inherited' radio. I wouldn't have guessed that :) I agree we should add the 'Inherited' option. Anyway, this design seems more user friendly for more general audience than mine so I will implement it. The only problem with it is that one have to come with new label for each group and empty value - can't be inferred from metadata. Is there any issue adding new labels at this point? Worst case we could hard code the label now and add a translation later. It might be better to use a composite widget of radio buttons and checkboxes so we can reuse the code. Probably the definition will look something like this: { name: 'ipakrbauthzdata', type: 'radio', Not sure if it should be radio, more like something new. Right, probably the current radio widget can't do this. So either we improve the radio widget or create something new. label: ..., options: [ { label: ..., value: 'NONE' }, { label: ..., type: 'checkboxes', Do you expect to be there something different than checkboxes, or do you want it to do it this way for possible future customization. Ideally it should be generic enough to combine any widgets. This might be a common scenario somewhere else: Something: ( ) Option 1 ( ) Option 2 (o) Other: [something else ] -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE
On 02/12/2013 05:14 PM, Endi Sukma Dewata wrote: On 2/8/2013 7:27 AM, Petr Vobornik wrote: Checkbox for NONE option was added. https://fedorahosted.org/freeipa/ticket/3404 Patches for master and 3.1 branch attached. ACK. We were discussing to NACK this approach. The implementation should be improved because of the mutually exclusive nature of NONE option with [MS-PAC, PAD] options. I think we should add spec definition (to Web UI only, or into server plugin as well) of mutex options. Something like: mutex_groups: [[['NONE'],['MS-PAC', 'PAD']], [/*another array of groups*/]]; basically an array of group arrays where group array would contain arrays of mutually exclusive option. Is it over-complicated? Would one array or a pair of groups be enough? That way we can disable and uncheck(should not happen, but to keep data consistency) checkboxes of options from other groups when user selects a value of some group. As a separate issue, we might want to fetch the options from metadata, when we want to show the values and don't care about creating different labels. -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE
On 2/12/2013 10:56 AM, Petr Vobornik wrote: We were discussing to NACK this approach. The implementation should be improved because of the mutually exclusive nature of NONE option with [MS-PAC, PAD] options. I think we should add spec definition (to Web UI only, or into server plugin as well) of mutex options. Something like: mutex_groups: [[['NONE'],['MS-PAC', 'PAD']], [/*another array of groups*/]]; basically an array of group arrays where group array would contain arrays of mutually exclusive option. Is it over-complicated? Would one array or a pair of groups be enough? That way we can disable and uncheck(should not happen, but to keep data consistency) checkboxes of options from other groups when user selects a value of some group. If they are mutually exclusive, they probably should be separated using radio buttons like this: PAC: ( ) None (o) Type: [x] MS-PAC [ ] PAD It might be better to use a composite widget of radio buttons and checkboxes so we can reuse the code. Probably the definition will look something like this: { name: 'ipakrbauthzdata', type: 'radio', label: ..., options: [ { label: ..., value: 'NONE' }, { label: ..., type: 'checkboxes', options: [ { label: ..., value: 'MS-PAC' }, { label: ..., value: 'PAD' } ] } ] } The composite widget will handle setting the appropriate widgets when the value of the field on load. It will handle enabling/disabling the checkboxes when the radio button is selected. It will also compute the final value of the field from selected radio button/checkboxes on save. Or just dim (no disable) and uncheck. That way there would still be visual distinction and one don't have to uncheck all the options from one group when he wants to select options of mutex group. That is also possible, but it changes the normal behavior of checkboxes, so probably users would have to play around with it to understand the grouping. They could also get confused, e.g. is dimmed checkbox disabled? As a separate issue, we might want to fetch the options from metadata, when we want to show the values and don't care about creating different labels. If we use radio buttons like above, new labels are necessary to describe the different groups of checkboxes. For all radio buttons checkboxes in general, if we don't want to use labels we probably could simplify the definition such that we can specify the string values directly (without nested object): { name: 'ipakrbauthzdata', type: 'radio', options: [ 'NONE', { type: 'checkboxes', options: ['MS-PAC', 'PAD'] } ] } -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel