Re: [Freeipa-devel] [PATCH] admiyo-0226-Added-a-container-for-the-facet

2011-05-02 Thread Endi Sukma Dewata

On 4/27/2011 9:39 PM, Endi Sukma Dewata wrote:

On 4/27/2011 8:47 PM, Adam Young wrote:

The more I think about it, the more I think that this structure can be
created by the navigation code, and then passed to the entity to
populate. The entity-header function moves to navigation.js, but does
not have a reference to the entity yet. When an entity tab gets
activated, we then populate this structure. everything can get created
on demand.


Not sure, I'd have to see how it's implemented. My concern is whether it
would limit entity customization.


This is my attempt at restructuring the DOM and the code:
http://edewata.fedorapeople.org/freeipa/install/ui/index.html

It's still work in progress, but the main changes are:
1. Navigation is modified to use entity names instead of numerical
   index.
2. Entity content is moved out of the navigation structure into a
   separate container.
3. Each facet now has separate container, so we don't need to
   redraw the page every time we open it.
4. Buttons are moved into facet header. They will appear under the
   facet tab groups.

The code is available in the navigation branch in this repository:
git://fedorapeople.org/~edewata/repo.git

--
Endi S. Dewata
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] admiyo-0226-Added-a-container-for-the-facet

2011-05-02 Thread Adam Young

On 05/02/2011 09:28 PM, Endi Sukma Dewata wrote:

On 4/27/2011 9:39 PM, Endi Sukma Dewata wrote:

On 4/27/2011 8:47 PM, Adam Young wrote:

The more I think about it, the more I think that this structure can be
created by the navigation code, and then passed to the entity to
populate. The entity-header function moves to navigation.js, but does
not have a reference to the entity yet. When an entity tab gets
activated, we then populate this structure. everything can get created
on demand.


Not sure, I'd have to see how it's implemented. My concern is whether it
would limit entity customization.


This is my attempt at restructuring the DOM and the code:
http://edewata.fedorapeople.org/freeipa/install/ui/index.html

It's still work in progress, but the main changes are:
1. Navigation is modified to use entity names instead of numerical
   index.

Good, ver y good.  This will simplify relationships.


2. Entity content is moved out of the navigation structure into a
   separate container.
Interesting call.  I'm pretty sure I am in favor of it, but I'd like to 
hear your rartionale.



3. Each facet now has separate container, so we don't need to
   redraw the page every time we open it.
4. Buttons are moved into facet header. They will appear under the
   facet tab groups.

The code is available in the navigation branch in this repository:
git://fedorapeople.org/~edewata/repo.git


I'll wait until you give me the go ahead to review, but the DOM 
structure looks good.


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


[Freeipa-devel] [PATCH] admiyo-0226-Added-a-container-for-the-facet

2011-04-27 Thread Adam Young


From f4f5a644ffcc31c417c6d81998b52e2b17c8f0e3 Mon Sep 17 00:00:00 2001
From: Adam Young ayo...@redhat.com
Date: Wed, 27 Apr 2011 13:32:14 -0400
Subject: [PATCH] Added a container for the facet

---
 install/ui/details.js |2 +-
 install/ui/entity.js  |   24 ++--
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/install/ui/details.js b/install/ui/details.js
index 1d653c23752e7a4a6a5c87ae3bd5c3379768e05d..877142032c02e59d0d52c8fbe699bb932facb68d 100644
--- a/install/ui/details.js
+++ b/install/ui/details.js
@@ -573,7 +573,7 @@ IPA.details_refresh = function() {
 method: 'show',
 options: { all: true, rights: true }
 });
-
+
 if (IPA.details_refresh_devel_hook){
 IPA.details_refresh_devel_hook(that.entity_name,command,that.pkey);
 }
diff --git a/install/ui/entity.js b/install/ui/entity.js
index 9a9f05f23476f0b768e3d52d739f790011bdcaa2..ccf41c39eff0fe48fc4b8421c1b2b683f2ab89b0 100644
--- a/install/ui/entity.js
+++ b/install/ui/entity.js
@@ -189,7 +189,7 @@ IPA.entity = function (spec) {
 facet.entity_name = that.name;
 that.facets.push(facet);
 that.facets_by_name[facet.name] = facet;
-
+
 if (facet.facet_group){
 if (!that.facet_groups[facet.facet_group]){
 that.facet_groups[facet.facet_group] = [];
@@ -309,9 +309,21 @@ IPA.entity_setup = function (container) {
 }
 facet.entity_header = entity.header;
 
-entity.header.reset();
-facet.create_content(facet.entity_header.content);
-facet.setup(facet.entity_header.content);
+$('.facet_container',entity.header.content).css('display','none');
+
+var facet_container = $(facet.name +'_container', entity.header.content);
+if (!facet_container.length){
+facet_container = $('div/',{
+id:facet.name +'_container',
+'class':'facet_container'
+}).appendTo(facet.entity_header.content);
+entity.header.reset();
+facet.create_content(facet_container);
+facet.setup(facet_container);
+}else{
+facet_container.css('display','block');
+}
+
 entity.header.select_tab();
 facet.refresh();
 };
@@ -532,7 +544,8 @@ IPA.entity_header = function(spec){
 return that.facet_tabs;
 }
 function content(){
-that.content = $(div class='content'/);return that.content;
+that.content = $(div class='content'/);
+return that.content;
 }
 
 function entity_container() {
@@ -548,7 +561,6 @@ IPA.entity_header = function(spec){
 
 function reset(){
 that.buttons.empty();
-that.content.empty();
 }
 that.reset = reset;
 
-- 
1.7.4.4

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

Re: [Freeipa-devel] [PATCH] admiyo-0226-Added-a-container-for-the-facet

2011-04-27 Thread Endi Sukma Dewata

On 4/27/2011 12:56 PM, Adam Young wrote:




Currently we have this DOM structure:

div id='user' class='entity-container'
  div class='entity-header'
div class='entity-title'/
span class='action-controls'/
div class='entity-title'/
input id='pkey' type='hidden'/
span class='entity-search'/
div class='entity-container-user' class='entity-container'
  div class='entity-tabs'/
  div class='content'
div id='search_container' class='facet_container'
  /div
/div
  /div
/div

1. The nested entity-container is confusing and probably will make CSS 
adjustment harder. The entity content should be in the same level as 
entity header.


2. The _container suffix for the facet container name is unnecessary 
because it already has a facet_container class.


3. The use of id attribute could lead to conflicts. It's better to use a 
name attribute and a class. Querying the element can be done with this 
selector:


   $('.facet[name=search]', ...)

4. Rename action-controls to entity-controls for consistency.

5. Rename facet_container to facet-container for consistency.

6. The hidden pkey can be stored in the entity object instead of DOM.

The DOM structure will look like this:

div name='user' class='entity'
  div class='entity-header'
div class='entity-title'/
div class='entity-controls'/
div class='entity-search'/
div class='entity-tabs'/
  /div
  div class='entity-content'
 div name='search' class='facet'
   div class='facet-header'/
   div class='facet-content'/
 /div
 div name='details' class='facet'/
  /div
/div

What do you think? We can do this in a separate patch if it's too 
complicated.


--
Endi S. Dewata
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] admiyo-0226-Added-a-container-for-the-facet

2011-04-27 Thread Adam Young

On 04/27/2011 09:09 PM, Adam Young wrote:

On 04/27/2011 04:39 PM, Endi Sukma Dewata wrote:

On 4/27/2011 12:56 PM, Adam Young wrote:




Currently we have this DOM structure:

div id='user' class='entity-container'
div class='entity-header'
div class='entity-title'/
span class='action-controls'/
div class='entity-title'/
input id='pkey' type='hidden'/
span class='entity-search'/
div class='entity-container-user' class='entity-container'
div class='entity-tabs'/
div class='content'
div id='search_container' class='facet_container'
/div
/div
/div
/div

1. The nested entity-container is confusing and probably will make 
CSS adjustment harder. The entity content should be in the same level 
as entity header.


2. The _container suffix for the facet container name is unnecessary 
because it already has a facet_container class.


3. The use of id attribute could lead to conflicts. It's better to 
use a name attribute and a class. Querying the element can be done 
with this selector:


   $('.facet[name=search]', ...)

4. Rename action-controls to entity-controls for consistency.

5. Rename facet_container to facet-container for consistency.

6. The hidden pkey can be stored in the entity object instead of DOM.

The DOM structure will look like this:

div name='user' class='entity'
div class='entity-header'
div class='entity-title'/
div class='entity-controls'/
div class='entity-search'/
div class='entity-tabs'/
/div
div class='entity-content'
div name='search' class='facet'
div class='facet-header'/
div class='facet-content'/
/div
div name='details' class='facet'/
/div
/div

What do you think? We can do this in a separate patch if it's too 
complicated.





Very clean:  I like it a lot.  I'd like to do it as a separate patch, 
just to keep clear what we are doing.


The more I think about it, the more I think that this structure can be 
created by the navigation code, and then passed to the entity to 
populate. The entity-header function moves to navigation.js, but does 
not have a reference to the entity yet.  When an entity tab gets 
activated, we then populate this structure.  everything can get created 
on demand.


I'm assuming that entity-search is for the search bar, and not the 
search facet:  search facet still goes inside the entity-content, right?





___
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


Re: [Freeipa-devel] [PATCH] admiyo-0226-Added-a-container-for-the-facet

2011-04-27 Thread Endi Sukma Dewata

On 4/27/2011 8:09 PM, Adam Young wrote:

div name='user' class='entity'
div class='entity-header'
div class='entity-title'/
div class='entity-controls'/
div class='entity-search'/
div class='entity-tabs'/
/div
div class='entity-content'
div name='search' class='facet'
div class='facet-header'/
div class='facet-content'/
/div
div name='details' class='facet'/
/div
/div

What do you think? We can do this in a separate patch if it's too
complicated.


Very clean: I like it a lot. I'd like to do it as a separate patch, just
to keep clear what we are doing.


Found some issues:

1. The selector on entity.js:314 is missing a # sign:

   var facet_container = $('#'+facet.name +'_container',
   entity.header.content);

2. Entity header is not updated after switching facets.
   Open Users tab, click one of the users, then click Back to Search.
   The title and buttons are wrong and the facet tabs do not disappear.
   This is because entity header is shared with other facets and isn't
   updated if the facet container already exists.

--
Endi S. Dewata
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel