Skip to content

wrap any plain text column headers in a <span>#117

Merged
texodus merged 2 commits intofinos:masterfrom
telamonian:wrap-txt-in-col-headers
Mar 16, 2021
Merged

wrap any plain text column headers in a <span>#117
texodus merged 2 commits intofinos:masterfrom
telamonian:wrap-txt-in-col-headers

Conversation

@telamonian
Copy link
Copy Markdown
Contributor

As discussed with @texodus, this PR changes the creation of column header nodes such that any text is wrapped up in a <span> element before getting added. As opposed to now, where any text is just included as a raw text node.

@telamonian
Copy link
Copy Markdown
Contributor Author

telamonian commented Mar 9, 2021

This PR also fixes an API consistency issue w.r.t. what is a valid value for a column_header entry. Previously, you could pass an html string such as "<span></span>" and it would automagically get rendered into actual html nodes before getting added to the column_header. On the other, if you tried passing in eg an actual live <span> element as a column_header, it would not get handled correctly and you would instead end up with a raw text node containing nonsense (something like "[HTMLSpan object]").

On the other hand, row_header values have the exact opposite handling (<span> element works, "<span></span>" wil result in a raw text node). This PR brings the handling behavior of column_header values in line with the current handling of row_header values.

All of which is to say that we should probably consider doing a v0.3.0 version bump before releasing this change, just in case anyone is relying on the exactly wonky details of the current column_header value handling.

@telamonian
Copy link
Copy Markdown
Contributor Author

telamonian commented Mar 15, 2021

All of the unittests now pass locally. A bunch of tests needed fixes w.r.t. the fact that column-headers are now rendered slightly differently. Most fixes were trivial, except for the sort-handling in the perspective example. The previous mousedown handler was unable to deal with clicking on a <span> child of a <th> column-header, and could only handle clicks directly on the <th> element itself. This is now fixed

@texodus Can you please review?

@texodus
Copy link
Copy Markdown
Member

texodus commented Mar 16, 2021

Thanks for the PR! Looks good!

@texodus texodus merged commit b891996 into finos:master Mar 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants