It appears to me that option 1 would be better prepared to be extensible ... That is if a plugin needed to add an action or a column, we could make that happen with pattern 1 (possibly after adding in a service) I'm not sure how plugins ever add these things with pattern 2.
> On Aug 20, 2015, at 1:41 PM, "Thai Q Tran" <tqt...@us.ibm.com> wrote: > > Hi Lin, > > Let me draw on some examples to help clarify what I mean. > > Option 1: > > table.controller.js > -------------------- > ctrl.headers = { > gettext('column 1'), > gettext('column 2') > }; > > ctrl.noItemMessage = gettext('You no longer have any items in your table. You > either lack the sufficient priveledges or your search criteria is not valid'); > > ctrl.batchActionList = [ > { name: 'create', onclick: somefunction, etc.... } > { name: 'delete', onclick: somefunction, etc.... } > ]; > > ctrl.rowActionList = [ > { name: 'edit', onclick: somefunction, etc.... } > { name: 'delete', onclick: somefunction, etc.... } > ]; > > table.html > ----------- > <div ng-controller="table.controller.js as ctrl"> > <horizon-table > headers="ctrl.headers" > batch-actions="ctrl.batchActionList" > row-actions="ctrl.rowActionList"> > </horizon-table> > </div> > > So now your controller is polluted with presentation and translation logic. > In addition, we will have to live with long gettext messages and add eslint > ignore rules just to pass it. The flip side is that you do have a simple > directive that points to a common template sitting somewhere. It is not that > much "easier" to the example below. What we're really doing is defining the > same presentation logic, but in the HTML instead. Lastly, I'll bring up the > customization again because many products are going to want to customize > their tables. They maybe the minority but that doesn't mean we shouldn't > support them. > > Option 2: > > table.html > ------------ > <table ng-controller="table.controller.js as ctrl"> > <thead> > <tr> > <action-list> > <action callback="someFunc" translate>Create</action> > <action callback="someFunc" translate>Delete</action> > </action-list> > </tr> > <tr> > <th translate>Column 1</th> > <th translate>Column 2</th> > </tr> > </thead> > <tbody> > <tr ng-repeat="items in ctrl.items"> > <td>....</td> > <td><action-list> > <action callback="someFunc" translate>Edit</action> > <action callback="someFunc" translate>Delete</action> > </action-list></td> > </tr> > </tbody> > </table> > > Here, your table.controller.js worries ONLY about data and data manipulation. > The presentation logic all resides in the HTML. If I want to add icons in the > table header, I can do that easily. Remember that this is plain HTML, this is > a lot easier for someone new to come in and learn this than our special > horizon-table directive. It is definitely easier to USE, but I would argue > that it is harder to learn. > > -------------- > > If you compare the two options above, you'll see that all we've really done > is move presentation logic from the controller into the HTML. You have to > define that logic somewhere, why not in the HTML? This makes it easier to > read and know what you're going to see in the browser (something HTML5 spec > is evangelizing), and you get the bonus benefit of customization. > > I'd like to point out that we aren't getting rid of directives, we're still > using directives them (like <action-list>, <action>, <magic-search>, etc..) > in our tables. The pattern is, you build your panels using smaller components > instead of having one giant component that encapsulates everything. Of > course, there isn't a right or wrong answer, in fact there are two very > different implementations of a table directive out there right now: > > http://ng-table.com (more inline with option 1) > http://lorenzofox3.github.io/smart-table-website/ (more inline with option 2) > > Basically, what I'm trying to say is: let's build something simple and easy > to understand first (small components that we work), then we can build > something more complex on top of it so that it easier to use. I don't think > there is a right or wrong answer, just two very different ways of thinking > and implementation. But if we start with smaller components first, we get the > goods of both world. The guys that want to customize will have a way to do it > by bypassing the horizon-table directive, and the guys that just want a > simple table can use the more complex directive. > > -----Lin Hua Cheng <os.lch...@gmail.com> wrote: ----- > To: "OpenStack Development Mailing List (not for usage questions)" > <openstack-dev@lists.openstack.org> > From: Lin Hua Cheng <os.lch...@gmail.com> > Date: 08/19/2015 05:15PM > Subject: Re: [openstack-dev] [Horizon] Update on Angular Identity work > > Hi Thai, > > Thanks for investigating the two options. > > Option 2 might be better. Folks have to learn the new pattern of writing > multiple files, so I think the learning curve for a new table directive is > not that much of a difference. > > I think option 2 is going to be easier to maintain, since we have a layer of > abstraction. It might even also increase adoptability since it would be > easier to use. It might be harder to customize, but that would probably not > be done often. The table directive would be used as is most of the time. > > My thought is design the code to be easy to use for the use case that will be > used most of the time rather than the customization case which maybe harder > to do. Which leads me to preferring option 2. > > Thanks, > Lin > >> On Wed, Aug 19, 2015 at 12:16 PM, Thai Q Tran <tqt...@us.ibm.com> wrote: >> Hi Lin, >> >> I agree with you and Eric that we have a lot of HTML fragments. Some of them >> I think make sense as directives: >> The table footer is a good example of something we can convert into a >> directive: https://review.openstack.org/#/c/207631/ >> The table header on the other hand is something more specific to your table: >> https://github.com/openstack/horizon/blob/master/openstack_dashboard/dashboards/identity/static/dashboard/identity/users/table/table-header.html >> >> So there are two approaches we can take here: >> 1. Keep some of the presentation related data in the HTML: mainly things >> like table headers, column definitions, translated texts, etc... I like this >> approach a bit more because it allow us to read the HTML and know exactly >> what we are expecting to see. This table.html is compose of smaller >> directives like hz-table-footer and regular html tags like <th> and <td> >> etc... I think as we have more of these smaller directives available, we can >> combine the fragments into one file. >> >> 2. We could create a more general table directive with a common template. >> This is more inline with what we have currently for legacy. BUT the >> presentation logic like translations, definitions would now have to reside >> in the table controller AND we lose the semantic readability part. Doing it >> this way could potentially introduce more complexity as it now requires >> people to learn the table directive, which could be very complex if it does >> not use smaller directives. Another common problem we encountered with this >> pattern was a lack of customization. In legacy, it was pretty hard to add an >> icon into a table cell. If we go down this route, I believe we might start >> to encounter the same issues. >> >> In summary, we are working on addressing the HTML fragments, but I think we >> as a community should go with option 1 and stay away from option 2. >> >> -----Lin Hua Cheng <os.lch...@gmail.com> wrote: ----- >> To: "OpenStack Development Mailing List (not for usage questions)" >> <openstack-dev@lists.openstack.org> >> From: Lin Hua Cheng <os.lch...@gmail.com> >> Date: 08/18/2015 02:36PM >> Cc: Vince Brunssen/Austin/IBM@IBMUS >> Subject: Re: [openstack-dev] [Horizon] Update on Angular Identity work >> >> >> I think the table setup pattern have some opportunity for reducing code >> duplication before it gets re-used by other panels.. >> >> We used to just need to write one file to define a table, now we have to >> write 9 files [1]. Can we have a table directive to reduce the duplicated >> code before moving forward to other panels? >> >> -Lin >> >> [1] >> https://github.com/openstack/horizon/tree/master/openstack_dashboard/dashboards/identity/static/dashboard/identity/users/table >> >>> On Tue, Aug 18, 2015 at 11:49 AM, Thai Q Tran <tqt...@us.ibm.com> wrote: >>> Hi everyone, >>> >>> Just wanted to keep everyone up to date on the angular panels work. The >>> goal was to set a pattern that others can follow, to that end, there were a >>> few requirements: >>> 1. reusable and possibly pluggable >>> 2. easy to understand >>> 3. reduce code duplication >>> >>> These requirements don't always go hand-in-hand, and that is the primary >>> reason why it is taking a bit longer. I believe we are nearing the end of >>> it, here are some items remaining that I believe is crucial to finishing up >>> this work. >>> >>> a. i18n was completed, so we need help moving gettext blobs to HTML >>> templates (example patch: https://review.openstack.org/#/c/210366/ ) >>> volunteers are welcomed! We want others to use the translate directive as >>> the main way to translate text blobs, this was why we went down this road >>> using babel and angular_extractor plugin. >>> >>> b. transfer table supports clone feature ( >>> https://review.openstack.org/#/c/211345/ ). There were a lot of template >>> duplications, this clone feature reduces the HTML by a considerable amount. >>> Since this is something we use quite often, it made sense to invest time >>> into improving it. We have had complaints that there was too much HTML >>> fragments, this will address a bit of that. One of the challenge was to get >>> it working with existing launch-instance, so I spent about 2 weeks making >>> sure it worked well with the old code while allowing the new clone feature. >>> >>> c. I believe we have a pretty good pattern setup for tables. The final >>> piece of the puzzle was the patterns for various actions. We have wizard >>> (create user), form (edit user), confirmation dialog (delete user), and >>> actions with no modal dialog (enable user). We wanted a general pattern >>> that would address the requirements mentioned above. There were some >>> discussions around extensibility at the midcycle that I think will fit well >>> here as well ( >>> https://blueprints.launchpad.net/horizon/+spec/angular-workflow-plugin ). >>> The actions can follow a similar pattern to workflow. I believe this >>> pattern would address #1 and #3 but making it easy to understand is a bit >>> challenging - I think this is where documentation could help. >>> >>> https://review.openstack.org/#/c/202315/ and a few other patches are going >>> to be ready for review soon (sometime end of this week)! Item #c is the >>> most important piece, it is going to be the general pattern that people >>> will use to build their angular panels with, so the more eyes we can get on >>> it, the better. My aim is to get it in before the feature freeze and I >>> think that is entirely possible with your help. So please help review even >>> if you are not a core! >>> >>> Thanks >>> >>> >>> >>> >>> __________________________________________________________________________ >>> OpenStack Development Mailing List (not for usage questions) >>> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe >>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >> >> __________________________________________________________________________ >> OpenStack Development Mailing List (not for usage questions) >> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >> >> >> __________________________________________________________________________ >> OpenStack Development Mailing List (not for usage questions) >> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > > __________________________________________________________________________ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > > __________________________________________________________________________ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
__________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev