You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by "Kamran Kashanian (JIRA)" <de...@myfaces.apache.org> on 2008/08/22 23:16:44 UTC

[jira] Created: (TRINIDAD-1189) Tree Expand All Handling

Tree Expand All Handling
------------------------

                 Key: TRINIDAD-1189
                 URL: https://issues.apache.org/jira/browse/TRINIDAD-1189
             Project: MyFaces Trinidad
          Issue Type: Improvement
          Components: Components
    Affects Versions: 1.2.9-core
         Environment: All
            Reporter: Kamran Kashanian
         Attachments: expandall.1.2.9.1.patch, expandall.trunk.1.2.x.patch

Submitting a patch for the issue below. (I assume three +1 votes for the proposal means approval)


There is some code in the Trinidad UIXTree which is intended to prevent the user from doing an unbounded "Expand All" on large trees (see UIXTree.broadcast() method. Specifically HierarchyUtils.__handleBroadcast()).

The code counts the number of newly expanded nodes and if it is > 100 then it counts the total number of nodes in the tree and if the total is also > 100, it prevents an expand all and only expands two levels.

There are several issues with this:

1) This code is executed on every expand/collapse event and can be expensive. Counting 100 nodes in the RowKeySet and in the tree (see TableUtils._getSizeOfTree()) can force the model to do additional data fetches beyond what is currently displayed in the view port. Also the number 100 is completely arbitary.

2) There is no way to for this code to know if the user did an "Expand All" or just did something to cause 100 nodes to expand. For example "Expand All Below", or just select 100 nodes and do "Expand" from a menu. In these cases the user will get unexpcted results (only two levels will expand)

3) Preventing the user from doing an Expand All should really be part of application logic and has no place in the framework. Normally the application would display a warning and ask the user if they really want to do an expand all in response to a user action


[Proposal] 

I would like to propose that this code be removed from UIXTree. I will submit a patch if there are no strong objections.  This *is* a change in the Trinidad tree functionality, and I am not sure how it impacts current users.


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (TRINIDAD-1189) Tree Expand All Handling

Posted by "Kamran Kashanian (JIRA)" <de...@myfaces.apache.org>.
     [ https://issues.apache.org/jira/browse/TRINIDAD-1189?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kamran Kashanian updated TRINIDAD-1189:
---------------------------------------

    Status: Patch Available  (was: Open)

> Tree Expand All Handling
> ------------------------
>
>                 Key: TRINIDAD-1189
>                 URL: https://issues.apache.org/jira/browse/TRINIDAD-1189
>             Project: MyFaces Trinidad
>          Issue Type: Improvement
>          Components: Components
>    Affects Versions: 1.2.9-core
>         Environment: All
>            Reporter: Kamran Kashanian
>         Attachments: expandall.1.2.9.1.patch, expandall.trunk.1.2.x.patch
>
>
> Submitting a patch for the issue below. (I assume three +1 votes for the proposal means approval)
> There is some code in the Trinidad UIXTree which is intended to prevent the user from doing an unbounded "Expand All" on large trees (see UIXTree.broadcast() method. Specifically HierarchyUtils.__handleBroadcast()).
> The code counts the number of newly expanded nodes and if it is > 100 then it counts the total number of nodes in the tree and if the total is also > 100, it prevents an expand all and only expands two levels.
> There are several issues with this:
> 1) This code is executed on every expand/collapse event and can be expensive. Counting 100 nodes in the RowKeySet and in the tree (see TableUtils._getSizeOfTree()) can force the model to do additional data fetches beyond what is currently displayed in the view port. Also the number 100 is completely arbitary.
> 2) There is no way to for this code to know if the user did an "Expand All" or just did something to cause 100 nodes to expand. For example "Expand All Below", or just select 100 nodes and do "Expand" from a menu. In these cases the user will get unexpcted results (only two levels will expand)
> 3) Preventing the user from doing an Expand All should really be part of application logic and has no place in the framework. Normally the application would display a warning and ask the user if they really want to do an expand all in response to a user action
> [Proposal] 
> I would like to propose that this code be removed from UIXTree. I will submit a patch if there are no strong objections.  This *is* a change in the Trinidad tree functionality, and I am not sure how it impacts current users.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (TRINIDAD-1189) Tree Expand All Handling

Posted by "Matthias Weßendorf (JIRA)" <de...@myfaces.apache.org>.
     [ https://issues.apache.org/jira/browse/TRINIDAD-1189?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Matthias Weßendorf updated TRINIDAD-1189:
-----------------------------------------

       Resolution: Fixed
    Fix Version/s: 1.0.10-core
                   1.2.10-core
         Assignee: Matthias Weßendorf
           Status: Resolved  (was: Patch Available)

> Tree Expand All Handling
> ------------------------
>
>                 Key: TRINIDAD-1189
>                 URL: https://issues.apache.org/jira/browse/TRINIDAD-1189
>             Project: MyFaces Trinidad
>          Issue Type: Improvement
>          Components: Components
>    Affects Versions: 1.2.9-core
>         Environment: All
>            Reporter: Kamran Kashanian
>            Assignee: Matthias Weßendorf
>             Fix For: 1.2.10-core, 1.0.10-core
>
>         Attachments: expandall.1.2.9.1.patch, expandall.trunk.1.2.x.patch
>
>
> Submitting a patch for the issue below. (I assume three +1 votes for the proposal means approval)
> There is some code in the Trinidad UIXTree which is intended to prevent the user from doing an unbounded "Expand All" on large trees (see UIXTree.broadcast() method. Specifically HierarchyUtils.__handleBroadcast()).
> The code counts the number of newly expanded nodes and if it is > 100 then it counts the total number of nodes in the tree and if the total is also > 100, it prevents an expand all and only expands two levels.
> There are several issues with this:
> 1) This code is executed on every expand/collapse event and can be expensive. Counting 100 nodes in the RowKeySet and in the tree (see TableUtils._getSizeOfTree()) can force the model to do additional data fetches beyond what is currently displayed in the view port. Also the number 100 is completely arbitary.
> 2) There is no way to for this code to know if the user did an "Expand All" or just did something to cause 100 nodes to expand. For example "Expand All Below", or just select 100 nodes and do "Expand" from a menu. In these cases the user will get unexpcted results (only two levels will expand)
> 3) Preventing the user from doing an Expand All should really be part of application logic and has no place in the framework. Normally the application would display a warning and ask the user if they really want to do an expand all in response to a user action
> [Proposal] 
> I would like to propose that this code be removed from UIXTree. I will submit a patch if there are no strong objections.  This *is* a change in the Trinidad tree functionality, and I am not sure how it impacts current users.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.