4 Replies Latest reply on Sep 9, 2014 2:43 AM by mtomasek

    Page Fragment API review

    bleathem

      Hello rich faced devs!

       

      In reviewing a recent PR for RF-13301:" Favor use of Page Fragments

      in Framework Tests" I've gotten a good chance to see page fragments

      in action.  I really like how they remove a much of the DOM setup of the

      integration tests.  A powerful abstraction indeed.

       

      However I'm a little concerned over some of the generic names we have in

      the page fragments API.  Consider the RichFacesTabPanel fragment in

      the above PR.  It has methods (among others) with the following names:

       

      • RichFacesTabPanel#getRootOfContainerElement() to return the content

      element of the TabPanel component

      • RichFacesTabPanel#getSwitcherControllerElements() to get the headers

      of the parent Tab

       

      Both of these names are quite generic since they are inherited from a

      parent class that shared amongst all switchable components.  However,

      for both these methods the super method is abstract, so there is no

      shared implementation.  Can someone with more knowledge of the page

      fragments implementation explain the benefit of the method inheritance

      here?  IMO the abstraction leaves us with an unclear API which won't

      help adoption.

       

      If we can come to an agreement of how we can improve this, we can scan

      the remaining page fragments for methods that can benefit from a similar

      naming improvement.

       

      Also, while on the subject of page fragments, I noticed the javadoc for

      page fragments is not included in our generated javadoc.  This also will

      hurt adoption.  I've created a jira RF-13816 to address including

      the javadoc for page fragments.  Please comment there if you feel they

      shouldn't be included.

       

      Cheers,

      Brian Leathem

       

      https://github.com/richfaces/richfaces/pull/108

      https://issues.jboss.org/browse/RF-13301

      https://github.com/richfaces/richfaces/blob/master/build/page-fragments/src/main/java/org/richfaces/fragment/tabPanel/RichFacesTabPanel.java

      https://issues.jboss.org/browse/RF-13816