[HTML5 Article Renderer] Refactor Table Implementation #14002
Conversation
github-actions
bot
added
DEV: renderers
Build Artifacts
|
rtibbles
left a comment
rtibbles
left a comment
There was a problem hiding this comment.
A bit more cleanup I think we can do.
There was a problem hiding this comment.
As this prop is no longer being used, I think we can just delete it?
There was a problem hiding this comment.
You mean tableCounter? because I can't see it from this comment
There was a problem hiding this comment.
No - the stylingOverrides prop.
There was a problem hiding this comment.
Likewise, we can delete styleOverrides here too?
There was a problem hiding this comment.
I think the prop being consumed by SafeHtmlImage
| return { | ||
| functional: true, | ||
| render(h) { | ||
| const tableChildren = []; |
There was a problem hiding this comment.
Now that we're just adding safe-html class to the caption - couldn't we just use the passed in mapChildren function here to map the children and then return them - because it already does that!
| const vm = this; | ||
| return { | ||
| functional: true, | ||
| render(h) { |
There was a problem hiding this comment.
This implementation looks correct, but architecturally it introduces a potential source of confusion. We’re creating a functional component inside a computed property of a regular component—essentially building a mini component-factory within the component. This adds layers of complexity we’re trying to move away from.
A cleaner alternative would be to generate the table elements dynamically (not programmatically) and then assigning relevant styling defined within the component itself, thus the redundancy of the style override props.
Our goal is to migrate the current functional component (.js) into an Options API or, ideally, a composable-based component structure. This will help keep the codebase more consistent and maintainable going forward.
akolson
left a comment
akolson
left a comment
There was a problem hiding this comment.
Hi Allan! Great start to this refactor. I left a comment that I think is important in better shaping the outcomes of this pr. Thanks
3 participants
Summary
This PR restructures the SafeHtmlTable as a Vue single-file component.
closes #13850
References
#13850
Reviewer guidance
Please see and verify ;