Re: Detailed review of 3.18.2. The datagrid element

On Tue, 11 Sep 2007, Mihai Sucan wrote:
> 
> 1. In the section 3.18.2.4.1. "Common default data provider method 
> definitions for cells" [2]:
> 
> Editable cells are defined such that "if the first element child of a 
> cell's element is an input element that has a type attribute with the 
> value text or that has no type attribute at all, then the cell acts as 
> an editable cell."
> 
> That's fine, however, it does ommit other important editable input 
> types: password, email, and url.
> 
> There are other editable input types, yet, I am not sure if these should 
> also be included: datetime, datetime-local, date, month, week, time, 
> number, and range.

Since the datagrid mechanism only supports plain text editing in this 
version, the other types are not supported for this purpose. A future 
version may allow other types to be edited.


> 2. I'm being "picky"  about the language/wording again. In section 3.18.2.5.
> "Populating the datagrid element" [3] one can read:
> 
> "... the element must fire a single load event at itself ..."
> 
> "The datagrid must then populate itself ..."
> 
> "... the datagrid must invoke the initialize() method ... "
> 
> The provided quotes share something in common: the datagrid does 
> everything. The datagrid fires events, the datagrid populates itself, 
> the datagrid invokes methods. Does it really do that? :)

As it says in the conformance section:

# Some conformance requirements are phrased as requirements on elements, 
# attributes, methods or objects. Such requirements fall into two 
# categories: those describing content model restrictions, and those 
# describing implementation behavior. The former category of requirements 
# are requirements on documents and authoring tools. The second category 
# are requirements on user agents.


> 3. In the same section [3] one can read:
> 
> "To establish how the type of a cell"
> 
> That's obviously wrong. Maybe that should be:
> 
> "To establish the cell type"

Fixed.


> 4. Again, in the same section [3], one can read:
> 
> "Examine the classes that apply to the cell. If the progress class 
> applies to the cell, it is a progress bar. Otherwise, if the cyclable 
> class applies to the cell, it is a cycling cell whose value can be 
> cycled between multiple states. Otherwise, none of these classes apply, 
> and the cell is a simple text cell."
> 
> Given the nature of classes, there might be "progress" and "cyclable". 
> What should the UA do then?

If the progress class applies to the cell, it is a progress bar.


> 5. Reading 3.18.2.7. "Requirements for interactive user agents" [4] I 
> found this:
> 
> "If a cell is a cell whose value can be cycled between multiple states, 
> then the user must be able to activate the cell to cycle its value. When 
> the user activates this "cycling" behaviour of a cell, then the datagrid 
> must invoke the data provider's cycleCell() method, with a 
> RowSpecification object representing the cell's row as the first 
> argument and the cell's column index as the second. The datagrid must 
> act as if the datagrid's updateCellChanged() method had been invoked 
> with those same arguments immediately *before* the provider's method was 
> invoked."
> 
> Why invoke updateCellChanged() before cycleCell()?
>
> From the definition of cycleCell() [5]:
> 
> "Called by the datagrid when the user changes the state of a cyclable 
> cell on row row, column column. The data provider should change the 
> state of the cell to the new state, as appropriate. There is no need to 
> tell the datagrid that the cell has changed, as the datagrid 
> automatically assumes that the given cell will need updating."
> 
> The last phrase is important: the datagrid automatically assumes that 
> the given cell *will* need updating - no need to tell the datagrid the 
> cell changed.
> 
> So, again, why *before*?
>
> The same "error" is made when defining editable cells and cells with a 
> checkbox. Maybe I'm not understanding something?
> 
> Yet... in the section 3.18.2.9. "Columns and captions" [6] one can read:
> 
> "If a column is sortable, then the user must be able to invoke it to 
> sort the data. When the user does so, then the datagrid must invoke the 
> data provider's toggleColumnSortState() method, with the column's index 
> as the only argument. The datagrid must *then* act as if the datagrid's 
> updateEverything() method had been invoked."
> 
> The keyword is *then*, which means the updateEverything() method is 
> invoked after the toggleColumnSortState() runs - as I would expect.
> 
> So, I might not be wrong.

You were not wrong as far as I can tell. Fixed.


> 6. "Big Issue: One possible thing to be added is a way to detect when a 
> row/selection has been deleted, activated, etc, by the user (delete key, 
> enter key, etc)."
> 
> Obviously this is needed. Maybe this would be best solved with some new 
> events fired on the datagrid element. I would suggest not to add any 
> method to the DataGridDataProvider which would be called for a specific 
> event. E.g. don't define a new method like DataGridDataProvider.onAdd(), 
> but instead make it an event "add". The event should include several 
> properties: a reference to the datagrid element, the new row, etc.
> 
> The reason I am suggesting the definition of new events is that one 
> would not need to define his/her own DataGridDataProvider if the default 
> one is good enough for his/her initial purpose - coupled with a few 
> event listeners.

I'd like to see an implementation of what exists in the spec now before 
adding more features like this.


> 7. I would like to know if the author can call the methods from the 
> default DataGridDataProvider?

There's no way to get a hold of that object, no.


> I was thinking: why not define datagrid.data such that it holds by 
> default all the methods as callable read-only native functions - one 
> cannot overwrite them individually, just the entire DataGridDataProvider 
> object. Would this be workable?
>
> This would be needed by scripts in applications that do not need to 
> overwrite the native object, yet need to get the row count, or get the 
> column count, etc.

I don't really understand the use case.


> 8. Continuing within a similar line of thought, the DataGridDataProvider 
> object should also include getRow(r) and getCell(r, c), to get the r-th 
> row element, and c-th cell element from the r-th row.
> 
> The datagrid is similar in concept with a table element. It would need 
> some methods to add/find/delete rows and columns - similar to the 
> provided methods on the table element.
> 
> These methods would not be needed in cases when a <table> is used as the 
> data provider for the datagrid. Yet, in the other cases (for selects, 
> datalists, and h1-h6, LIs, etc) ... this set of methods would be a 
> welcome addition.

I guess we can look at adding this in a later version, but for now they 
seem like "nice to have" stuff, not really necessary.


> 9. I would have a general comment on this element: it's too complex, 
> trying to do it all. "Simple" concepts as the <table> element ended up 
> being quite very complex implementation-wise, specially when used 
> wrongly (e.g. tables used for layouts). Perhaps the spec leaves a lot of 
> "dark holes" noticeable only when one starts to implement this element, 
> and when authors start putting the element into real usage. Probably 
> there are lots of places where UA implementations can differ.
> 
> What I find that opens lots of "back-doors" is the content model itself, 
> allowing: select elements, datalist elements, table elements, and 
> headings (h1-h6) with list items included. That's "woaaah": overly 
> complex - too many ways to make a single datagrid.
> 
> Let's not forget the "do-it-all" way to make a datagrid: the 
> DataGridDataProvider object, which by itself does everything everyone 
> would ever wish (or so it seems).
> 
> This is just general rant :) - general concerns. I can't suggest 
> anything much. Wouldn't it have been better to split this datagrid into 
> multiple elements, each being "specialised" for certain use-cases? Not 
> having all the use-cases in mind, like now.

The default data grid might be too much, I agree. It will be interesting 
to see how implementors find it. I don't see that the rest of it is 
especially complicated though.

-- 
Ian Hickson               U+1047E                )\._.,--....,'``.    fL
http://ln.hixie.ch/       U+263A                /,   _.. \   _\  ;`._ ,.
Things that are impossible just take longer.   `._.-(,_..'--(,_..'`-.;.'

Received on Monday, 28 July 2008 23:45:10 UTC