[PLAYER-103] refacto default turn icon and tooltip#179
Conversation
jparez
force-pushed
the
dev/player-103-refacto-default-turn-icon-and-tooltip
branch
2 times, most recently
from
d88e69e to
f2108bb
Compare
pgivel
left a comment
pgivel
left a comment
There was a problem hiding this comment.
small comment, LGTM otherwise
| */ | ||
| setTooltip(element, text, position, classes = null) { | ||
| setTooltip(element, text, position, classes = null, isHTML = false) { | ||
| // isHTML is used to display HTML content in the tooltip. It should be used with caution, especially if the text content is user-generated (translation, ...). |
There was a problem hiding this comment.
This comment feels a bit redundant with the docstring right above
There was a problem hiding this comment.
True, but I'm not sure developers will read the JSDoc, especially with a clearly named param. I prefer to keep the warning, but i can merge the 'beware' from the jsdoc into this sentence and clean up the jsdoc. Let me know what you think
There was a problem hiding this comment.
as you wish, I don't have a strong opinion on this :)
There was a problem hiding this comment.
After reflection, I prefer to keep both. Currently, the JSDoc isn't very useful because objects are marked as 'any', but I'd like to add TypeScript to the project later. In that case, JSDoc will be very helpful. This way, we’ll have a visual indicator (like an alert emoji) on hover, as well as the warning inside the function code for future updates.
jparez
force-pushed
the
dev/player-103-refacto-default-turn-icon-and-tooltip
branch
from
f2108bb to
781e0d7
Compare
…ewport constraints
…ton and remove associated styles
jparez
force-pushed
the
dev/player-103-refacto-default-turn-icon-and-tooltip
branch
from
781e0d7 to
0b18ae5
Compare
…t-turn-icon-and-tooltip [PLAYER-103] refacto default turn icon and tooltip
2 participants
Description
refacto default turn icon and tooltip
Tested on a PaaS device (injected the built .js and .css files).
without connectionFailedURL option :
with connectionFailedURL option :