Re: [Freeipa-devel] [PATCHES] 124-132 Inconsistent ways to show/change entry status
On 05/14/2012 07:08 PM, Endi Sukma Dewata wrote: On 5/10/2012 7:19 AM, Petr Vobornik wrote: Updated patch attached. See comments below. On 05/08/2012 04:23 AM, Endi Sukma Dewata wrote: Try adding a very long DNS zone, then open the zone. Compare the breadcrumbs in the DNS Resource Records page and in the Settings page, in my case the second one is a bit longer. I think the length of the breadcrumb should be calculated differently. I remade how breadcrumb is limited. Now it tries to use as much space available with keeping low complexity level of calculation. It still uses an average but now the average is calculated in two phases in order to neglect a presence of short keys. ACK. Pushed to master. -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 124-132 Inconsistent ways to show/change entry status
On 5/10/2012 7:19 AM, Petr Vobornik wrote: Updated patch attached. See comments below. On 05/08/2012 04:23 AM, Endi Sukma Dewata wrote: Try adding a very long DNS zone, then open the zone. Compare the breadcrumbs in the DNS Resource Records page and in the Settings page, in my case the second one is a bit longer. I think the length of the breadcrumb should be calculated differently. I remade how breadcrumb is limited. Now it tries to use as much space available with keeping low complexity level of calculation. It still uses an average but now the average is calculated in two phases in order to neglect a presence of short keys. ACK. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 124-132 Inconsistent ways to show/change entry status
Updated patch attached. See comments below.
On 05/08/2012 04:23 AM, Endi Sukma Dewata wrote:
On 5/3/2012 8:26 AM, Petr Vobornik wrote:
On 05/03/2012 03:19 PM, Petr Vobornik wrote:
I found that limitation of maximum pkey length in facet header is not
working well. Attaching patch #134 which actually calculates it.
I found useless line in the patch. Corrected version attached.
Try adding a very long DNS zone, then open the zone. Compare the
breadcrumbs in the DNS Resource Records page and in the Settings page,
in my case the second one is a bit longer. I think the length of the
breadcrumb should be calculated differently.
I remade how breadcrumb is limited. Now it tries to use as much space
available with keeping low complexity level of calculation. It still
uses an average but now the average is calculated in two phases in order
to neglect a presence of short keys.
Btw, I just noticed that the facet tab label for DNS Resource Records is
changed to 'Search'. Same problem happens in Automount.
New ticket: #2744, patch on the list.
--
Petr Vobornik
From cfc37cb8cf8a823bc21207bab5722d17fbf63b55 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Thu, 3 May 2012 14:42:01 +0200
Subject: [PATCH] Improved calculation of max pkey length in facet header
Very long pkeys in facet header were limited to 60 characters. This magic number was good enough but with new action lists it isn't.
This patch is adding calculation of maximum characters for pkey in facet header. It fixes regression introduced by Action Lists and also it uses effectively available space.
Also this patch is changing limiting of breadcrumbs element to use as much space as possible. It works in three steps. First a threshold is set which is equal to length average. Then a total length of keys with length less than threshold is calculated. From this we can get remaining space for long keys and calculate new threshold. At last keys are limited to new threshold.
https://fedorahosted.org/freeipa/ticket/2247
f
---
install/ui/facet.js | 64 +++---
1 files changed, 55 insertions(+), 9 deletions(-)
diff --git a/install/ui/facet.js b/install/ui/facet.js
index 64d4f5d2adaafbce08d571060cb0fd723b10e558..31a9df73c289cb591434254d37d4a5f424469d74 100644
--- a/install/ui/facet.js
+++ b/install/ui/facet.js
@@ -369,17 +369,21 @@ IPA.facet_header = function(spec) {
if (!value) return;
-var limited_value = IPA.limit_text(value, 60);
+var key, i;
+var pkey_max = that.get_max_pkey_length();
+var limited_value = IPA.limit_text(value, pkey_max);
if (!that.facet.disable_breadcrumb) {
var breadcrumb = [];
+var keys = [];
var entity = that.facet.entity.get_containing_entity();
while (entity) {
+key = IPA.nav.get_state(entity.name+'-pkey');
breadcrumb.unshift($('a/', {
'class': 'breadcrumb-element',
-text: IPA.nav.get_state(entity.name+'-pkey'),
+text: key,
title: entity.metadata.label_singular,
click: function(entity) {
return function() {
@@ -390,17 +394,34 @@ IPA.facet_header = function(spec) {
}));
entity = entity.get_containing_entity();
+keys.unshift(key);
}
+//calculation of breadcrumb keys length
+keys.push(value);
+var max_bc_l = 140; //max chars which can fit on one line
+var max_key_l = (max_bc_l / keys.length) - 4; //4 chars as divider
+var bc_l = 0;
+var to_limit = keys.length;
+
+//count how many won't be limited and how much space they take
+for (i=0; ikeys.length; i++) {
+var key_l = keys[i].length;
+if (key_l = max_key_l) {
+to_limit--;
+bc_l += key_l + 4;
+}
+}
+
+max_key_l = ((max_bc_l - bc_l) / to_limit) - 4;
+
+
that.path.empty();
-var key_max_lenght = 60 / breadcrumb.length;
-for (var i=0; ibreadcrumb.length; i++) {
+for (i=0; ibreadcrumb.length; i++) {
var item = breadcrumb[i];
-
-var entity_key = item.text();
-var limited_entity_key = IPA.limit_text(entity_key, key_max_lenght);
-item.text(limited_entity_key);
+key = IPA.limit_text(keys[i], max_key_l);
+item.text(key);
that.path.append(' raquo; ');
that.path.append(item);
@@ -408,10 +429,12 @@ IPA.facet_header = function(spec) {
that.path.append(' raquo; ');
+key = IPA.limit_text(keys[i], max_key_l);
+
$('span', {
'class':
Re: [Freeipa-devel] [PATCHES] 124-132 Inconsistent ways to show/change entry status
The code works, it's ACKed. I have some comments below:
On 5/2/2012 8:33 AM, Petr Vobornik wrote:
This bunch of patches are implementing ticket #2247. They introduce some
new logic and types of internal objects. There might be design issues
(mainly in state evaluation). I would appreciate some opinions on what
might be improved.
See patch comments for more details.
What I think might be the main concerns:
Action list definition:
Now action lists are defined on facet level and facet header takes them
from facet. Would in be better to define action list - the widget and
actions separately. The widget could be defined in header and actions on
facet level.
Yes, the header could be considered a widget, so it contain the action
list widget.
State evaluation:
The patches are adding support for some kind of state evaluation. The
state is represented by array of string. I'm thinking that the design
might not be robust. Is a string good enough? We might have a problem
with conditions like to have this particular access right' (#2318).
There are state_evaluators and state_listeners. They are practically the
same but the execution point and parameters are different. Should they
be somehow joined?
It might be better to use a map to represent the state. The map could
hold any attribute types, not just string flag with the current array.
So, the widget itself could be the state because it's also a map. That
said, evaluating a map might be difficult to define declaratively, we
would end up using a code again. So for now using an array of string is
fine, but we just have to know the limitations.
Code placement:
There's a lot of new objects and for some of them it is not clear to
what code file they should be placed.
This will continue to change as we add more code. Feel free to create a
new file when you see a pattern. We might want to start separating the
IPA-specific code from the framework (reusable code). The framework
could be moved into a 'lib' folder.
FYI: In close future I would like to address some problems in UI
architecture. I'm in a middle of designing phase, so there is nothing to
present at the moment. The main topics are:
* reduce the need of overriding methods when a new widgets or
capabilities are added
* make it more declarative to enhance extendibility
It may be done by:
* better inheriting model to support events
* build phases (preinit, init, postinit, create, load) to improve spec
object creating and initialization of created object.
* path representation of an event/attribute/model property to support
bindings to various events/attrs from anywhere
* introduce model and model bindings, converters between command
output/model/human readable representation
Sounds good! Some comments about the patch:
1. When you open a specific user, there's a default action that's
already selected in the action list. The thing is the current default
action (i.e. Disable) is probably not something that's regularly used.
It might be better to show something like '-- Select Action --' or '--
Action List --' so you'd have to select an action first then click Apply.
2. Suppose we did #1, do we still need the confirmation when applying
the action?
3. I think the action list should be available in all details page for
all entities. At least it should have the Delete action.
4. Something to think about, do we need an action list in the
association page? Entities like group default to an association page
instead of details page. So if we don't have an action list in the
association page the UI may look inconsistent because you'd have to go
to the Settings to execute an action. But if we do have an action list
in the association page, the actions could become confusing: do they
apply to the associations or to the entry itself? One possible solution
is to move the Settings to the left-most position and make it the
default page and have the action list in the details page only.
5. Some details pages have status indicator (check sign or minus sign)
on the left of the page title, some others don't. This is because not
all entities have a concept of status. Would it be better to show the
status as a read-only field in the facet content?
6. It might be better to avoid using element ID in the CSS. This would
make the CSS more reusable.
#content .facet-action-list div[name=apply] a.ui-state-default {
...
}
7. In some facet class definitions the no_init parameter is defined
separately from the spec object, any particular reason?
IPA.facet = function(spec, no_init) {
...
};
You can think of spec object as named parameters. So the no_init can be
defined in the spec object and later used to determine what operations
to be done inside the init method.
8. The declarative control button definitions still contain some code,
e.g. the handler function inside the button actions. Maybe the actions
should be defined in a separate list and the buttons can
Re: [Freeipa-devel] [PATCHES] 124-132 Inconsistent ways to show/change entry status
On 5/3/2012 8:26 AM, Petr Vobornik wrote: On 05/03/2012 03:19 PM, Petr Vobornik wrote: I found that limitation of maximum pkey length in facet header is not working well. Attaching patch #134 which actually calculates it. I found useless line in the patch. Corrected version attached. Try adding a very long DNS zone, then open the zone. Compare the breadcrumbs in the DNS Resource Records page and in the Settings page, in my case the second one is a bit longer. I think the length of the breadcrumb should be calculated differently. Btw, I just noticed that the facet tab label for DNS Resource Records is changed to 'Search'. Same problem happens in Automount. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 124-132 Inconsistent ways to show/change entry status
I found that limitation of maximum pkey length in facet header is not
working well. Attaching patch #134 which actually calculates it.
On 05/02/2012 03:33 PM, Petr Vobornik wrote:
This bunch of patches are implementing ticket #2247. They introduce some
new logic and types of internal objects. There might be design issues
(mainly in state evaluation). I would appreciate some opinions on what
might be improved.
See patch comments for more details.
What I think might be the main concerns:
Action list definition:
Now action lists are defined on facet level and facet header takes them
from facet. Would in be better to define action list - the widget and
actions separately. The widget could be defined in header and actions on
facet level.
State evaluation:
The patches are adding support for some kind of state evaluation. The
state is represented by array of string. I'm thinking that the design
might not be robust. Is a string good enough? We might have a problem
with conditions like to have this particular access right' (#2318).
There are state_evaluators and state_listeners. They are practically the
same but the execution point and parameters are different. Should they
be somehow joined?
Code placement:
There's a lot of new objects and for some of them it is not clear to
what code file they should be placed.
FYI: In close future I would like to address some problems in UI
architecture. I'm in a middle of designing phase, so there is nothing to
present at the moment. The main topics are:
* reduce the need of overriding methods when a new widgets or
capabilities are added
* make it more declarative to enhance extendibility
It may be done by:
* better inheriting model to support events
* build phases (preinit, init, postinit, create, load) to improve spec
object creating and initialization of created object.
* path representation of an event/attribute/model property to support
bindings to various events/attrs from anywhere
* introduce model and model bindings, converters between command
output/model/human readable representation
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
--
Petr Vobornik
From a53c1ed5ad21c274d9563ea1f6c7bf95757e8ea7 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Thu, 3 May 2012 14:42:01 +0200
Subject: [PATCH] Improved calculation of max pkey length in facet header
Very long pkeys in facet header were limited to 60 characters. This magic number was good enough but with new action lists it isn't.
This patch is adding calculation of maximum characters for pkey in facet header. It fixes regression introduced by Action Lists and also it uses effectively available space.
https://fedorahosted.org/freeipa/ticket/2247
---
install/ui/facet.js | 28 +++-
1 files changed, 27 insertions(+), 1 deletions(-)
diff --git a/install/ui/facet.js b/install/ui/facet.js
index 64d4f5d2adaafbce08d571060cb0fd723b10e558..30bf8798a0446d4e3057d09bce16a14f87f688a5 100644
--- a/install/ui/facet.js
+++ b/install/ui/facet.js
@@ -369,7 +369,8 @@ IPA.facet_header = function(spec) {
if (!value) return;
-var limited_value = IPA.limit_text(value, 60);
+var pkey_max = that.get_max_pkey_length();
+var limited_value = IPA.limit_text(value, pkey_max);
if (!that.facet.disable_breadcrumb) {
var breadcrumb = [];
@@ -580,6 +581,31 @@ IPA.facet_header = function(spec) {
that.adjust_elements();
};
+that.get_max_pkey_length = function() {
+
+var label_w, max_pkey_w, max_pkey_l, al, al_w, title_max_w, icon_w,
+char_w, container_w;
+
+container_w = that.container.width();
+char_w = that.title_widget.title.css('font-size');
+icon_w = that.title_widget.icon.width();
+label_w = that.title_widget.title.width();
+char_w = label_w / that.title_widget.title.text().length;
+max_pkey_w = container_w - icon_w - label_w;
+max_pkey_w -= 10; //some space correction to be safe
+
+if (that.action_list) {
+al = that.action_list.container;
+al_w = al.width();
+
+max_pkey_w -= al_w;
+}
+
+max_pkey_l = Math.ceil(max_pkey_w / char_w);
+
+return max_pkey_l;
+};
+
that.adjust_elements = function() {
if (that.action_list) {
--
1.7.7.6
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 124-132 Inconsistent ways to show/change entry status
On 05/03/2012 03:19 PM, Petr Vobornik wrote:
I found that limitation of maximum pkey length in facet header is not
working well. Attaching patch #134 which actually calculates it.
I found useless line in the patch. Corrected version attached.
On 05/02/2012 03:33 PM, Petr Vobornik wrote:
This bunch of patches are implementing ticket #2247. They introduce some
new logic and types of internal objects. There might be design issues
(mainly in state evaluation). I would appreciate some opinions on what
might be improved.
See patch comments for more details.
What I think might be the main concerns:
Action list definition:
Now action lists are defined on facet level and facet header takes them
from facet. Would in be better to define action list - the widget and
actions separately. The widget could be defined in header and actions on
facet level.
State evaluation:
The patches are adding support for some kind of state evaluation. The
state is represented by array of string. I'm thinking that the design
might not be robust. Is a string good enough? We might have a problem
with conditions like to have this particular access right' (#2318).
There are state_evaluators and state_listeners. They are practically the
same but the execution point and parameters are different. Should they
be somehow joined?
Code placement:
There's a lot of new objects and for some of them it is not clear to
what code file they should be placed.
FYI: In close future I would like to address some problems in UI
architecture. I'm in a middle of designing phase, so there is nothing to
present at the moment. The main topics are:
* reduce the need of overriding methods when a new widgets or
capabilities are added
* make it more declarative to enhance extendibility
It may be done by:
* better inheriting model to support events
* build phases (preinit, init, postinit, create, load) to improve spec
object creating and initialization of created object.
* path representation of an event/attribute/model property to support
bindings to various events/attrs from anywhere
* introduce model and model bindings, converters between command
output/model/human readable representation
___
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
--
Petr Vobornik
From 6a547d2e8202f0295d2e3d9ad7dd3fb6ac98ebe4 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Thu, 3 May 2012 14:42:01 +0200
Subject: [PATCH] Improved calculation of max pkey length in facet header
Very long pkeys in facet header were limited to 60 characters. This magic number was good enough but with new action lists it isn't.
This patch is adding calculation of maximum characters for pkey in facet header. It fixes regression introduced by Action Lists and also it uses effectively available space.
https://fedorahosted.org/freeipa/ticket/2247
---
install/ui/facet.js | 26 +-
1 files changed, 25 insertions(+), 1 deletions(-)
diff --git a/install/ui/facet.js b/install/ui/facet.js
index 64d4f5d2adaafbce08d571060cb0fd723b10e558..b5ec95952eeb32050b45b0dc97a017b6cd652435 100644
--- a/install/ui/facet.js
+++ b/install/ui/facet.js
@@ -369,7 +369,8 @@ IPA.facet_header = function(spec) {
if (!value) return;
-var limited_value = IPA.limit_text(value, 60);
+var pkey_max = that.get_max_pkey_length();
+var limited_value = IPA.limit_text(value, pkey_max);
if (!that.facet.disable_breadcrumb) {
var breadcrumb = [];
@@ -580,6 +581,29 @@ IPA.facet_header = function(spec) {
that.adjust_elements();
};
+that.get_max_pkey_length = function() {
+
+var label_w, max_pkey_w, max_pkey_l, al, al_w, icon_w, char_w, container_w;
+
+container_w = that.container.width();
+icon_w = that.title_widget.icon.width();
+label_w = that.title_widget.title.width();
+char_w = label_w / that.title_widget.title.text().length;
+max_pkey_w = container_w - icon_w - label_w;
+max_pkey_w -= 10; //some space correction to be safe
+
+if (that.action_list) {
+al = that.action_list.container;
+al_w = al.width();
+
+max_pkey_w -= al_w;
+}
+
+max_pkey_l = Math.ceil(max_pkey_w / char_w);
+
+return max_pkey_l;
+};
+
that.adjust_elements = function() {
if (that.action_list) {
--
1.7.7.6
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
