cyrilou242 commented on pull request #6467: URL: https://github.com/apache/incubator-pinot/pull/6467#issuecomment-772816083
> The dimensionPlaceHolderId is a dom identifier which is just a name. From the naming conversion, it has already used kebab-case. Why not replace '\' to '-'? Thanks for reviewing. I'm not sure to understand your point so I'll give more details about my MR. The div name is defined here: https://github.com/apache/incubator-pinot/blob/2be1520d99206f1c606a9062544f88f958ff1da0/thirdeye/thirdeye-frontend/app/pods/components/heatmap-chart/template.hbs#L9 `dimName` can contain some dots. I'm not trying to change the div name, and an id value containing a dot is _in theory_ perfectly fine. The fact is that in the js logic, an id value containing a dot is a problem at the following lines: https://github.com/apache/incubator-pinot/blob/b6c7a971032b1da51a05b78531d5bf4ea9f048b4/thirdeye/thirdeye-frontend/app/pods/components/heatmap-chart/component.js#L109 jquery requires proper dot escaping, ref: https://stackoverflow.com/questions/9930577/jquery-dot-in-id-selector https://github.com/apache/incubator-pinot/blob/b6c7a971032b1da51a05b78531d5bf4ea9f048b4/thirdeye/thirdeye-frontend/app/pods/components/heatmap-chart/component.js#L121 D3 requires proper dot escaping, ref: https://stackoverflow.com/questions/33502614/d3-how-to-select-element-by-id-when-there-is-a-dot-in-id So my idea was to implement the escaping in the js logic, not to change the dom identifier. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
