I have just a few concerns about this patch, some are about the patch itself and some are about what it tries to achieve :D
First of all I would request you to split the patch, it's better to have one patch for each achievement. It makes easier to review and merge them individually. In this specific case I would merge the New enable/disable change but I'm not sure that making all the crud thing bootstrap coupled is a great idea. On TG you are not required to use bootstrap, the fact that is the default CSS framework when quickstarting doesn't mean that you cannot live without it. Many past projects actually are not on bootstrap at all. The current issue with CSS frameworks is that they enforce coupling between the structure (HTML) and the look (CSS), you end having classes/id that describe how that thing will look instead of what it is. This is probably related to the fact that CSS frameworks are somehow a recent concept which is still immature and are just starting to evolve in the semantic direction using LESS, SASS mixins or similar things. Semantic grids are a recent concept and it will take some time before full css frameworks will embrace the concept in an effective manner. The style option was a good way to make possible for people to customize the look and feel of the crud controllers without forcing any particular solution and providing a default look (which was backward compatible with projects before 2.1.5). I think that support for bootstrap should be achieved in some other way instead of coupling the crud extension to bootstrap itself. Probably a good solution might be to make possible to add custom classes to the tags/widgets like ToscaWidgets does and let people add classes for the css framework they want to use. We can then provide a BootstrapCrudRestController which is already configured with classes for bootstrap. Just the first idea that came to my mind, my open to other solutions, I just think that crud itself probably shouldn't even know that bootstrap exists if not explicitly stated. On Sat, May 26, 2012 at 12:34 PM, Moritz Schlarb <[email protected]> wrote: > Hi there! > > I did some work to make tgext.crud use the various features of the > Twitter Bootstrap CSS framework. > Attached you will find a patch against the 0.5 tag (I wanted it to work > on TurboGears-2.1.5, using tgext.crud-0.5.1 just gives me > RenderingErrors related to Genshi). > > Also, I did only port the Mako templates, as Genshi complained about the > XML() function not being defined (and even if I replace it by Markup() > in the tgext.crud templates, tw2 uses XML() again...) > > Changes are: > - Display menu in a span2 column with highlighted current item > - Display New ${model} link as button with a Plus icon > - Optionally disable the "New" functionality altogether with a parameter > - Display rest of forms in a span10 on the right side. > > To do: > - Pagination... Bootstrap includes some pagination styles > (http://twitter.github.com/bootstrap/components.html#pagination) but > they require having the pager using lists which I haven't found possible > in webhelpers.paginate > - Use a nicer style for the action buttons in the table --> sprox > e.g. > <a ... class="btn btn-mini"><i class="icon-pencil"></i> Edit</a> > <input type="submit" class="btn-mini btn-danger" ... > value="Delete" /> > - Use the Bootstrap form elements --> tw2 > > Hope this patch can be useful in some way! > > Cheers, > Moritz > > -- > You received this message because you are subscribed to the Google Groups > "TurboGears Trunk" group. > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected]. > For more options, visit this group at > http://groups.google.com/group/turbogears-trunk?hl=en. > -- You received this message because you are subscribed to the Google Groups "TurboGears Trunk" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/turbogears-trunk?hl=en.
