On 12/09/2011 08:17 AM, Endi Sukma Dewata wrote:
On 12/8/2011 8:31 AM, Petr Vobornik wrote:
https://fedorahosted.org/freeipa/ticket/1890

This is an improvement, so it's ACKed. There are some other issues, they
can be done separately:

Pushed to master.

Attaching new patch which addresses following issues:

1. Create an HBAC service with a very long name. Open an HBAC rule, add
the service, it will widen the table. Same problem with sudo rule.

The table wasn't set as 'scrollable'. I didn't find a reason why cell width calculation should be different for scrollable and 'normal', so I unified it and it fixes the problem.

2. Open the HBAC service you created in #1, the facet header is too long
it messes up the tabs. The service name here should be trimmed as well.

The pkey in facet group header is now limited to 20 chars. It would be probably good enough for normal names. It may overlap with with next facet group if current facet group has 1 facet with short name, but I think it isn't a problem now.

3. Delete the HBAC service you created in #1 from command line. Reload
the page in #2, it will pop up an error dialog. The content of the
dialog should wrap as well.

Error dialog now uses word-wrap.

4. The limit_text() could be useful outside of IPA.facet_header, so it
can be made an independent function.

Moved to ipa.js.

5. The limit_text() returns a string up to max_length+3 because of the
additional '...'. It's better to include the '...' in the calculation so
the output is not longer than the expected max_length.

Fixed.
6. In widget.js:1104 what are the constants 4 and 14? Can they be combined?

Added comments to code. They shouldn't be combined because one applies only if the table is selectable. But thanks for pointing it out - there was actually a little error - the '14' should be in ?: statement and both of the weren't entirely correct. - Fixed

7. When a column has a width set, there is a line (widget.js:1098): width += 16; Why is it there? I'd rather subtract it.

--
Petr Vobornik
From ca2ce8099ef836d1d09949a111baef0823998e25 Mon Sep 17 00:00:00 2001
From: Petr Vobornik <pvobo...@redhat.com>
Date: Fri, 9 Dec 2011 14:13:17 +0100
Subject: [PATCH] Additional better displaying of long names

 - facet group headers, error dialog, non-scrollable tables, can manage long names

 Size calculation of scrollable and non-scrollable tables was united. Now these types of tables differ only by style.

https://fedorahosted.org/freeipa/ticket/1821
---
 install/ui/facet.js  |   23 ++++++-----------------
 install/ui/ipa.css   |   18 +++++++++++++-----
 install/ui/ipa.js    |   13 +++++++++++++
 install/ui/widget.js |   48 +++++++++++++++++++++---------------------------
 4 files changed, 53 insertions(+), 49 deletions(-)

diff --git a/install/ui/facet.js b/install/ui/facet.js
index df5743b1cf7ffe2d9df1905808f6e9dfc92be8ea..4128b0562fa8d3095cd746ab460ba4f887e05196 100644
--- a/install/ui/facet.js
+++ b/install/ui/facet.js
@@ -193,24 +193,11 @@ IPA.facet_header = function(spec) {
         }
     };
 
-    that.limit_text = function(value, max_length) {
-
-        if (!value) return '';
-
-        var limited_text = value;
-
-        if (value.length && value.length > max_length + 3) {
-            limited_text = value.substring(0, max_length)+'...';
-        }
-
-        return limited_text;
-    };
-
     that.set_pkey = function(value) {
 
         if (!value) return;
 
-        var limited_value = that.limit_text(value, 60);
+        var limited_value = IPA.limit_text(value, 60);
 
         if (!that.facet.disable_breadcrumb) {
             var breadcrumb = [];
@@ -234,13 +221,13 @@ IPA.facet_header = function(spec) {
             }
 
             that.path.empty();
-            var key_max_lenght = 60/breadcrumb.length;
+            var key_max_lenght = 60 / breadcrumb.length;
 
             for (var i=0; i<breadcrumb.length; i++) {
                 var item = breadcrumb[i];
 
                 var entity_key = item.text();
-                var limited_entity_key = that.limit_text(entity_key, key_max_lenght);
+                var limited_entity_key = IPA.limit_text(entity_key, key_max_lenght);
                 item.text(limited_entity_key);
 
                 that.path.append(' &raquo; ');
@@ -379,13 +366,15 @@ IPA.facet_header = function(spec) {
 
                 var label = facet_group.label;
                 if (pkey && label) {
-                    label = label.replace('${primary_key}', pkey);
+                    var limited_pkey = IPA.limit_text(pkey, 20);
+                    label = label.replace('${primary_key}', limited_pkey);
                 } else {
                     label = '';
                 }
 
                 var label_container = $('.facet-group-label', span);
                 label_container.text(label);
+                if (pkey) label_container.attr('title', pkey);
 
                 var facets = facet_group.facets.values;
                 for (var j=0; j<facets.length; j++) {
diff --git a/install/ui/ipa.css b/install/ui/ipa.css
index b4453df2bfc669905c7770ce5f6ad7e05f929e8e..703f37cf0b187419d6745022e2fde0c475c7a736 100644
--- a/install/ui/ipa.css
+++ b/install/ui/ipa.css
@@ -624,11 +624,18 @@ span.main-nav-off > a:visited {
     height: 100%;
 }
 
+.content-table thead {
+    position: absolute;
+    top: 0px;
+    left: 3px;
+    right: 3px;
+}
+
 .content-table tbody {
     position: absolute;
     top: 31px;
     left: 3px;
-    right: 4px;
+    right: 3px;
     bottom: 35px;
 }
 
@@ -895,17 +902,18 @@ a, .ui-widget-content a {
 /* ---- Dialog ---- */
 
 .ui-dialog .ui-dialog-titlebar-close span {
-        background-color: transparent !important;
+    background-color: transparent !important;
 }
 
 .ui-dialog .ui-dialog-content {
+    word-wrap: break-word;
     /* this should go away once we can fix table scrolling */
-    overflow:auto;
+    overflow: auto;
 }
 
 .ui-dialog .ui-dialog-titlebar {
-        padding: 0.5em 1em;
-        position: relative;
+    padding: 0.5em 1em;
+    position: relative;
 }
 
 .ui-dialog .ui-dialog-buttonpane button {
diff --git a/install/ui/ipa.js b/install/ui/ipa.js
index 6b0704606f6fa5e1ae7e9a30a305ccadb95f79ab..b39a4e566200179995b18fbef766ccee5603dfe8 100644
--- a/install/ui/ipa.js
+++ b/install/ui/ipa.js
@@ -1097,6 +1097,19 @@ IPA.error_list = function() {
     return that;
 };
 
+IPA.limit_text = function(value, max_length) {
+
+    if (!value) return '';
+
+    var limited_text = value;
+
+    if (value.length && value.length > max_length) {
+        limited_text = value.substring(0, max_length - 3)+'...';
+    }
+
+    return limited_text;
+};
+
 IPA.config = {
     default_priority: 500
 };
diff --git a/install/ui/widget.js b/install/ui/widget.js
index ffbcbadf16fef1192f1cbf4608f309756b78c398..69fe704b8e93adde78243adca18de16ce441ab06 100644
--- a/install/ui/widget.js
+++ b/install/ui/widget.js
@@ -1091,32 +1091,27 @@ IPA.table_widget = function (spec) {
 
             th = $('<th/>').appendTo(tr);
 
-            if (that.scrollable) {
-                var width;
-                if (column.width) {
-                    width = parseInt(
-                        column.width.substring(0, column.width.length-2),10);
-                    width += 16;
-                } else {
-                    /* don't use the checkbox column as part of the overall
-                       calculation for column widths.  It is so small
-                       that it throws off the average. */
-                    width = (that.table.width() - 4 -
-                             ((that.selectable ?
-                              IPA.checkbox_column_width : 0) + 14)) /
+            var width;
+            var cell_spacing = 16; //cell padding(2x6px), border (2x1px), spacing (2px)
+            if (column.width) {
+                width = parseInt(
+                    column.width.substring(0, column.width.length-2),10);
+                width += 16;
+            } else {
+                /* don't use the checkbox column as part of the overall
+                    calculation for column widths.  It is so small
+                    that it throws off the average. */
+                width = (that.thead.width() -
+                        2 - //first cell spacing
+                        ((that.selectable ? IPA.checkbox_column_width +
+                          cell_spacing : 0))) /
                         columns.length;
-                    width -= 16; //cell padding, border, spacing
-                }
-                width += 'px';
-                th.css('width', width);
-                th.css('max-width', width);
-                column.width = width;
-            } else {
-                if (column.width) {
-                    th.css('width', column.width);
-                    th.css('max-width', column.width);
-                }
+                width -= cell_spacing;
             }
+            width += 'px';
+            th.css('width', width);
+            th.css('max-width', width);
+            column.width = width;
 
             var label = column.label;
 
@@ -1131,9 +1126,7 @@ IPA.table_widget = function (spec) {
                     'style': 'float: right;'
                 }).appendTo(th);
             }
-            if (that.scrollable && !column.width){
-                column.width = th.width() +'px';
-            }
+
         }
 
         that.tbody = $('<tbody/>').appendTo(that.table);
@@ -1174,6 +1167,7 @@ IPA.table_widget = function (spec) {
                 width = parseInt(
                         column.width.substring(0, column.width.length-2),10);
                 width += 7; //data cells lack right padding
+                width += 'px';
                 td.css('width', width);
                 td.css('max-width', width);
             }
-- 
1.7.6.4

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to