Opened 8 years ago

Closed 3 years ago

#12820 closed defect (fixed)

can't parse HTML5 data-dojo-props on sub-nodes

Reported by: Kitson Kelly Owned by: Adam Peller
Priority: high Milestone: 1.11
Component: Dojox Version: 1.6.0
Keywords: html5, data-dojo-* Cc:
Blocked By: Blocking:

Description (last modified by bill)

When using declarative mode, the parser ignores data-dojo-props on any sub objects of the main declaration.

For example in the situation of a DataGrid, you might expect the following to work:

<table data-dojo-type="dojox.grid.DataGrid" data-dojo-props="store:exampleStore">
  <thead>
    <tr>
      <th data-dojo-props="field:'firstField',width:'40px'"></th>
      <th data-dojo-props="field:'secondField',width:'auto'"></th>
    </tr>
  </thead>
</table>

But in fact, you have to declare it like this:

<table data-dojo-type="dojox.grid.DataGrid" data-dojo-props="store:exampleStore">
  <thead>
    <tr>
      <th field="firstField" width="40px"></th>
      <th field="secondField" width="auto"></th>
    </tr>
  </thead>
</table>

This affects any dojo declarative object that requires sub tags within the declaration. This means that non-HTML5 compliant attributes are required in order to make the declarative items work.

Change History (10)

comment:1 Changed 8 years ago by bill

Component: ParserDojoX Grid
Description: modified (diff)
Owner: changed from bill to evan

The functionality you are talking about is part of DataGrid. The parser doesn't have any code for pulling values off of sub-nodes besides <script> tags. So changing component.

Note though that your statement that:

you have to declare it like this

is technically incorrect, you could presumably declare everything in top data-dojo-props field, like:

<table data-dojo-type="dojox.grid.DataGrid" data-dojo-props="
	store: test_store,
	structure: [[
	{name: 'Column 1', field: 'col1'},
	{name: 'Column 2', field: 'col2'},
	{name: 'Column 3', field: 'col3'}
]]">

comment:2 Changed 8 years ago by bill

Summary: dojo 1.6 Parser doesn't parse sub data-dojo-propscan't parse data-dojo-props on sub-nodes

I think the appropriate change to dojox.Grid would be to simply accept "data-dojo-field" on the <th> nodes, in addition to "field", if that isn't supported already.

comment:3 Changed 8 years ago by Kitson Kelly

Not only is the "field" attribute required, there are lots of other attributes, like "formatter" and "width" that are required to get the right configuration of a column in data grid. So that route would require quite an extensive list of data-dojo-xxx to support all the column attributes.

I think we should consider a consistent way of handling this. Because this does not only effect the declarative DataGrid?, but it also affects other widgets, the like dojox.charting where there are required sub-nodes. For example:

<div data-dojo-type="dojox.charting.widget.Chart2D" data-dojo-props="theme:dojox.charting.themes.PlotKit.blue">
  <div class="plot" type="Pie" fontColor="white" htmlLabels="false" radius="70"></div>
  <div class="series" id="Series A" name="Series A" store="fooStore" field="slice"></div>
  <div class="action" type="MoveSlice"></div>
  <div class="action" type="Tooltip"></div>
</div>

Again, many non-HTML5 attributes required to make the declarative work.

These modes of declarative follow the logic of using sub-nodes that mirror "parts" of the owning widget. Putting all the configuration in the main declaration essentially "breaks" the whole declarative model/benefits.

So I think there are two options:

  • There is a mechanism to offloading sub-node parsing to the parser, instead of being the responsibility of the main widget
  • Component designers follow the model of widgets like the ruler, where all parts of a complex widget are actually components of their own, like the Slider which has the slider as one object and the Rulers as separate components.

comment:4 Changed 8 years ago by bill

I agree that (eventually) the parser should be able to parse sub-nodes, I wrote about that in my somewhat outdated Dijit 2.0 plan.

In the meantime it should fairly easy for widgets (DataGrid, Charting, etc.) to look for both data-dojo-foo and foo with a simply utility function like

function get(node, name){
   return node.getAttribute("data-dojo-" + name) || node.getAttribute(name);
}

comment:5 Changed 8 years ago by Evan

Owner: changed from evan to Evan

comment:6 Changed 6 years ago by dylan

Component: DojoX GridDojox
Keywords: html5 data-dojo-* added
Milestone: tbd1.9
Owner: changed from Evan to Adam Peller
Summary: can't parse data-dojo-props on sub-nodescan't parse HTML5 data-dojo-props on sub-nodes

I suggest we fix this anywhere we possibly can in Dojo in time for 1.9 (not just on DataGrid?, but other things like charts and form manager, etc., which have tickets open on this).

I'm moving this to the general DojoX component because it covers a variety of areas in DojoX.

comment:7 Changed 6 years ago by Kitson Kelly

This was opened in my "ignorant" days. Honestly I think this might be a change too far for 1.9 to do this in a globally consistent fashion. This would require major changes to any component that utilises sub-nodes, changes to the parser and likely make it very difficult to maintain backwards compatibility.

Bill and I talked about it when we were doing the changes for the parser for 1.8 and we thought it was too difficult in the 1.X code base.

The only other option is for each owner to do as bill says and ensure they are dealing with it properly, but I am of the opinion a global "fix" in 1.X won't work.

comment:8 Changed 6 years ago by cjolif

Agreed with kitsonk. Each component owner should look into it and see how to fix it. I don't think a global fix for 1.9 is reasonable and we should better work on having a clean solution for 2.0. I'll try to come up with something for charting but I'm not confident at all this will make 1.9 as this was not planned at the beginning. Charting ticket is: #14648.

comment:9 Changed 6 years ago by Kitson Kelly

Milestone: 1.9future

comment:10 Changed 3 years ago by dylan

Milestone: future1.11
Resolution: fixed
Status: newclosed

Closing the global/meta ticket.

Note: See TracTickets for help on using tickets.