[seam-dev] Excel module added to SVN

[Nicklas Karlsson]

> * 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

OK on all.

> * 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:

Sounds good.

> * Avoid custom exception types - why do we need them?

The original idea was to separate what we throw from other runtime
exceptions. Besides, we didn't want
JExcelAPI specific exceptions in the Workbook interface and thought it
would be strange to just catch them
and pick an existing runtime exception and throw that.

> * 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.

OK. What about when referring to the file format? Should it be "The
Microsoft(R) Excel() spreadsheet application file format" (I think
it's actually called BIFF)?

How about 'The Microsoft(R) Excel(R) spreadsheet application,
hereafter called "E"'? ;-)

> * Also, there should be an 80 col gutter with the docs, and inline XML (like
> <literal>) should be placed inline

OK, I'll trim.

> * 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

Yep, it's actually a dummy warning, I'll fix it.

> * 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>

It comes from me mirroring the JExcelAPI a little too close, that
sounds resonable. I'll look into ui:fragment, I
don't think I've used it before.

>
> with the CSS for altering the styling
> * We should look at moving xthe worksheet "commands" to be inline (like
> html)

? Does not compute.

> * I remain totally unconvinced by the template stuff - what does this solve
> that facelets doesn't in a neater, consistent way?

You you elaborate on the facelets way? I was just looking for a way to
minimize typing but
I guess the css you mention below would take care of most of it(?)

> * 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

Is the PDF generation moving this way also or should they just take
separate paths?

> * The custom CSS names should be in line with standard css e.g.
>
> xlsFontName -> xls-font-family
> xlsFontSize -> xls-font-size

OK, reasonable.

> 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
> />)

Anywhere in the code I can look for how css link resources are
referenced? Any small
CSS parsing libraries out there? I figure it can get hairy really
quickly if the you support
both link-style and inline-style CSS and have normal and xls-style
attributes in both
styleClass and style and they are mixed down through the entire xhtml tree...

But it sure is worth a look.

> Sorry for the length of this :-)

It's nice you took the time to give feedback!