zhangloo333 commented on pull request #6467: URL: https://github.com/apache/incubator-pinot/pull/6467#issuecomment-773058892
> > 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. Thank you for your explanation. It looks good when the purpose is escaping in the js logic. ---------------------------------------------------------------- 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]
