On 10/24/2011 10:43 PM, Endi Sukma Dewata wrote:
On 10/21/2011 6:40 AM, Petr Vobornik wrote:
1) Wouldn't be better if the asterisk has different color than the
label? Visually I don't like it that much and I think it can be
overlook. Attaching a proposition. I used green IPAish color because red
usually means error.

Code from browser how it was done:

<td class="section-cell-label" title="First name"><label
name="givenname" class="field-label">First name:</label><span
class="required" style="
float: right;
font-weight: bold;
color: #319016;
font-size: 20px;
" title="required">*</span></td>

(style should be moved to css file)


<div style="line-height: 25px;"><span class="required" style="
font-weight: bold;
color: #319016;
font-size: 20px;
vertical-align: middle;
">*</span> required</div>

It may vary on the section type.

Fixed. I couldn't use the exact style above because it looks a bit weird
in the details page since the labels are right aligned. I use red for
now because it can also mean 'important', but we can adjust this again
later.

2) When rendering label, we should also obtain field input's id (if
possible) for 'for' attribute of <label>. This can be done separately.

I'd rather avoid using ID in a dynamic app like this. Adding ID into the
fields is possible, but we'd have to do it very carefully to avoid
conflicts.

3) Should we create some common pure html widgets with certain
semantics?

We can, but as discussed elsewhere, we have to fix the current
assumption first: each widget correspond to an attribute, it currently
has properties and methods (e.g. metadata, load(), save()) that are only
valid on attributes.

IE asterisk shouldn't be directly concatenated with label
text. It is used on more than one place which may cause maintenance
issues.

IPA.form(or some other name).required_indicator = function() {
return '*'
};

in this case this seems unnecessary. But if the required indicator was
like in 1) it will be useful.

Fixed.

Summary:
All 3 points are nice to have. If you think is not necessary then ACK.

This patch is also fixing https://fedorahosted.org/freeipa/ticket/1973 .

Noted in the new patch.


ACK

Also proposing minor visual enhancement (see attached patch).

--
Petr Vobornik

From e0a19768c2f3f4bf3d20e217ad878f44f921bfbb Mon Sep 17 00:00:00 2001
From: Petr Vobornik <pvobo...@redhat.com>
Date: Tue, 25 Oct 2011 15:53:06 +0200
Subject: [PATCH] Minor visual enhancement of required indicator

https://fedorahosted.org/freeipa/ticket/1696

Changes:
 * in details table facet '*' don't break colon alignment
 * bolder, bigger (-> IMHO nicer) asteriks
 * float (visual style) moved to css file
---
 install/ui/ipa.css   |   11 +++++++++++
 install/ui/widget.js |    2 +-
 2 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/install/ui/ipa.css b/install/ui/ipa.css
index be4ad361e6f9262a8ecb6a1744cce9cf25e29f1e..0652b375aec447166c34920ff190679a366886f6 100644
--- a/install/ui/ipa.css
+++ b/install/ui/ipa.css
@@ -1300,4 +1300,15 @@ body.info-page  {
 
 .required-indicator {
      color: red;
+     font-weight: bold;
+     font-size: 120%;
+}
+
+.section-cell-label .required-indicator {
+    float: right;
+    margin-right: -10px;
+}
+
+.dialog-section .section-cell-label .required-indicator {
+    margin-right: 0px;
 }
\ No newline at end of file
diff --git a/install/ui/widget.js b/install/ui/widget.js
index 1d3d31b38f12a8756d87e1b76aa3a294cfa96744..feaf7b2099e46624121b410c26f28f03179c2794 100644
--- a/install/ui/widget.js
+++ b/install/ui/widget.js
@@ -136,7 +136,7 @@ IPA.widget = function(spec) {
         that.required_indicator = $('<span/>', {
             'class': 'required-indicator',
             text: IPA.required_indicator,
-            style: 'display: none; float: right;'
+            style: 'display: none;'
         }).appendTo(container);
     };
 
-- 
1.7.6.4

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

Reply via email to