On Aug 1, 2008, at 1:56 AM, Daniel Roth wrote: Thanks - I'm going to look at it this morning...

Guys, This stuff is great, thank you. I've got a few comments: * ExcelExporter's namespace isn't the same as its package name (it should be) * Create an import for org.jboss.seam.excel in the META-INF/ components.xml of the module to allow reference as just excelExporter * Reuse the uiComponent Seam component to look up the provided datatable * All Seam components should be declared @BypassInterceptors * Why are the Workbook implementation components Seam components? And the factory design feels wrong to me. I would suggest a class based scheme, with the ability to register extra types via components.xml: @Name("org.jboss.seam.excel.excelFactory") @Scope(ScopeType.STATELESS) @AutoCreate @BypassInterceptors public class ExcelFactory { private static Map<String, Class<? extends ExcelWorkbook>> defaultImplementations; static { defaultImplementations = new HashMap<String, Class<? extends ExcelWorkbook>>(); defaultImplementations.put("csv", CsvExcelWorkbook.class); defaultImplementations.put("jxl", JXLExcelWorkbook.class); } private Map<String, Class<? extends ExcelWorkbook>> implementations; // getter and setter for implementations public static ExcelFactory instance() { return (ExcelFactory) Component.getInstance(ExcelFactory.class); } public ExcelWorkbook getExcelWorkbook(String type) { if (Strings.isEmpty(type)) { type = "jxl"; } Class<? extends ExcelWorkbook> clazz; if (implementations != null) { clazz = implementations.get(type); } else { clazz = defaultImplementations.get(type; } ExcelWorkbook excelWorkbook = clazz.newInstance(); if (excelWorkbook == null) { throw new IllegalArguementException("Unable to create workbook of type " + type_; } return excelWorkbook; } } * Avoid custom exception types - why do we need them? * We need to be careful in the documentation to always refer to "the Microsoft (R) Excel (R) spreadsheet application" (Microsoft (R) Excel(R) doesn't cut it) - so I added &excel; and &Excel; macros to ensure we do this. * Also, there should be an 80 col gutter with the docs, and inline XML (like <literal>) should be placed inline * I get these errors when running the example: 12:35:24,567 WARN [JXLTemplates] Could not find cell template data, try 12:35:24,568 WARN [JXLTemplates] Could not find cell template global, try * Why have <e:headerFooter type="header">? Why not <e:header />, <e:footer /> - this seems to make much more sense to me * What is the point of <e:headerFooterCommands>? To allow multiple children of a facet? Why not use <ui:fragment /> * Why this weird headerCommand stuff? Why not do this normally? <e:header> <f:facet name="left"> This worksheet was created at #{time} on #{date}. It has #{total_page} pages. </f:facet> </e:header> with the CSS for altering the styling * We should look at moving xthe worksheet "commands" to be inline (like html) * I remain totally unconvinced by the template stuff - what does this solve that facelets doesn't in a neater, consistent way? * The CSS stuff for the datatable exporter is excellent. This is the way formatting should be specified on the custom tags too, and probably remove all the formatting attributes from the e: tags * The custom CSS names should be in line with standard css e.g. xlsFontName -> xls-font-family xlsFontSize -> xls-font-size It would also be great if you were able to support compound values as CSS does (e.g. xls-font: 9pt sans-serif) and also use standard CSS attributes if they are present e.g. on the datatable exporter <h:dataTable style="font: sans-serif 9pt; xls-font-size: 12pt;" /> would produce a sans-serif 9pt font in HTML and a 12pt sans-serif font on the spreadsheet. We also need to support externally defined CSS (via <link />) Sorry for the length of this :-) Pete On 1 Aug 2008, at 07:56, Daniel Roth wrote:

On 5 Aug 2008, at 20:00, Nicklas Karlsson wrote: Yeah, that I guess is the endless debate - do you wrap the exception and make it easy for the user to catch that exception or do you reduce the complexity of stack traces and just pass on exceptions, and reduce clutter by using built in exception types. In general Seam goes with not wrapping and using existing exceptions. I would say, especially in the case of a UI control set, where nobody is likely to actually catch the exception, that you go with that where possible. How much is that going to require you to have public String getFoo() throws Exception; on your interface? I just looked again at the code and see you are swallowing exceptions - don't ever do that ;-) Is it trademarked? Hmm, I'm not sure this will be that useful on reflection. bar.xhtml: <ui:composition> <e:cell alignment="right"> <e:font name="Times New Roman"/> <e:background color="blue"/> <ui:insert /> </e:cell> </ui:composition> main.xhtml: <e:workbook xmlns:e="http://jboss.com/products/seam/excel"> <e:worksheet value="#{data}" var="item"> <e:column> <s:decorate template="bar.xhtml"> #{item.value} </s:decorate> </e:worksheet> </e:workbook> And CSS would make it neater still :-) That's a good point - all your examples show using the value attribute explictly, but does it work with just text as the child (implied as the value)? I would like the PDF stuff to go this way too as I think its much nicer. But that kinda depends on Norman ;-) When you investigate it for this, I would keep in mind that we would want to be able to use it both places. Yup, this is definitely the hardest part. But I think it is worth it. To me, what these JSF control sets (mail, pdf, excel and html) provide is unified way to create disparate types of output - so the idea is not just to mirror the Java API in XML (which is already easier) but to also make it a "seamless" experience. This is what I tried to do with mail - not just mirror the API (which would have been super ugly anyway as JavaMail is a pig), but to allow the user to write standard- JSF-like code but produce mail. How about investigating http://www.w3.org/Style/CSS/SAC/ - apparently it can parse standard CSS and allows you define your own selectors.

On 6 Aug 2008, at 07:16, Nicklas Karlsson wrote: No you are right, there is a good arguement for wrapping it to. I think then "the BIFF(R) file format" should be fine. Lets just be careful. Should do ;-) Look at the mail example for how.