Skip to content
This repository has been archived by the owner on Dec 12, 2018. It is now read-only.

Cell (get/formatter/renderCell) #12

Closed
pottedmeat opened this issue Mar 12, 2017 · 6 comments
Closed

Cell (get/formatter/renderCell) #12

pottedmeat opened this issue Mar 12, 2017 · 6 comments

Comments

@pottedmeat
Copy link
Contributor

pottedmeat commented Mar 12, 2017

Enhancement

In dgrid 1, the get function was used to return a text value and formatterwas used to return HTML - inserted in the standard cell wrapper. In addition to this, renderCell could be used to return your own DOM which would use the value of get but formatter wouldn't be called as part of this lifecycle.

get is a way to prevent HTML injection that may come from user data and formatter is a way to mark the HTML as potentially safe.

Ken, in his brain dump, noted that this decision for this API was mostly a legacy holdover . He proposed re-implementing these methods as renderCell, renderText, and renderHtml.

DNode already combines renderText and renderHtml (through the innerHTML property). Also, because dgrid 2 uses a registry, renderCell is accomplished instead by registering a different widget for 'cell'.

Our implementation should look like this:

  • If render is used, it is the only method that is called with item and column data and no value is found
  • If get is used, it is called with item and column data to find the value
  • Otherwise, the value is not used, a property lookup is performed on the item data
  • If renderValue is used, it is called with the value and the item and column data
  • The content created by render, renderValue, or a stringified value is passed to the cell so that the cell has both the value and the content
@msssk
Copy link
Contributor

msssk commented Mar 15, 2017

I'm not understanding how these concerns are irrelevant. Are you envisioning that the widget for cell renders the TD element, or only its content? In dgrid 1 the renderCell method is responsible for rendering the TD element, which provides complete control and flexibility at the cost of having to know how to correctly render a valid dgrid cell.

To avoid duplicating dgrid cell-rendering logic and keep the API user-friendly, simple use-cases are provided by single-purpose functions:

  • get: a way of generating a data value
  • formatter: a way of formatting provided data

Some way or another I think it is of value to continue providing these mechanisms in dgrid 2.

@pottedmeat
Copy link
Contributor Author

I was thinking the widget for "cell" renders the TD element while get/formatter could be handled by a callback that returns a DNode.

My point about irrelevancy is that because DNode can be a string (equivalent to get) or virtual DOM (equivalent to formatter) or a widget, they can be handled through the same function.

@msssk
Copy link
Contributor

msssk commented Mar 15, 2017

dgrid 1 currently has a lot of flexibility while managing to not get too complicated. One thing I think is important is the pipeline:

get -> formatter -> renderCell

get is single-purpose and lets you do type conversion (number parsing, date parsing) and generate calculated data values. You can implement get and not have to know anything about formatting or the dgrid cell structure.

formatter is single-purpose and lets you embed the provided data value (provided by get or direct from the store item) into your own HTML. You can implement formatter and not have to know anything about where the data value came from or the dgrid cell structure.

renderCell is the most flexible, and if you want you can do everything get and formatter do within this method. However, you can also leverage the results of get, and if desired you can manually invoke formatter in your renderCell implementation. renderCell receives the dgrid-generated TD element and it can directly manipulate it, including manipulating its attributes, or it can simply return a Node that dgrid will then set as the TD element's child. You don't need to know anything about the dgrid cell structure.

One thing I think is important is that dgrid always handles creating the required cell structure. None of these methods requires the user to re-implement dgrid logic (e.g. dgrid cell attributes, like class, role). And in the case of renderCell the user gets a reference to the dgrid cell, so if necessary they can modify it.

=====

So... if we have a single method that accepts a DNode:

  • can we do HTML formatting independently of data retrieval?
  • can we specify non-string data values? (number, Date, etc)

And for registering a different widget to render 'cell':

  • does this widget need to re-implement dgrid logic, like the node must be a TD, it must have role=gridcell, it must have class=dgrid-cell?

@msssk
Copy link
Contributor

msssk commented Mar 15, 2017

...sorry to be long-winded, I'm not attached to old ways for sentimentality, I just want to make sure we have a flexible and friendly API moving forward!

@pottedmeat
Copy link
Contributor Author

I think, then, the thing I left out from my initial analysis is that while get is tied to a specific column, formatter may be a much more reusable function - shared between columns.

I wasn't 100% happy with using formatter with a passed value. If this value is derived, it means the formatter should always implement get as well or be passed an unused garbage value which is then derived within formatter. It isn't the end of the world, but seems like it could be done better.

Perhaps a solution is to introduce a third option to handle this use case and introduce the following callbacks:

  • get: derives a value based on the item and column
  • render: creates a DNode (string, v call, v call with innerHTML, or w call)
  • renderValue: creates a DNode using a passed value either automatically derived using that item's field or the result of the get call.

@pottedmeat
Copy link
Contributor Author

To answer more of these questions, the callbacks should handle these situations:

  • A string inserted in the default td
  • HTML inserted in the default td
  • vdom inserted in the default td
  • A widget inserted in the default td

The only time you'd have to recreate the td, as with dgrid 1, is if you used a different widget for the "cell" entry in the grid's registry.

@dylans dylans added this to the 2017.03 milestone Mar 22, 2017
@dylans dylans modified the milestones: 2017.03, 2017.04 Apr 2, 2017
@dylans dylans modified the milestones: 2017.05, 2017.04 Apr 29, 2017
pottedmeat added a commit to pottedmeat/dgrid that referenced this issue May 22, 2017
@eheasley eheasley modified the milestones: 2017.05, 2017.06 Jun 6, 2017
@eheasley eheasley added beta3 and removed beta2 labels Jun 6, 2017
pottedmeat added a commit to pottedmeat/dgrid that referenced this issue Jun 6, 2017
Customize cell content, fixes dojo#12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants