-
Notifications
You must be signed in to change notification settings - Fork 213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Don't let data-{classPrefix} attribute override content option #134
Conversation
@TrevorBurnham I think the |
@TrevorBurnham yeah the PR missed docs sorry about that guys, I'm not sure I follow the tooltip About the DOM > JS by looking at the other PR I think the motivation was to keep same behavior as css inline styles but I'm fine either way, at least I don't see how it would bother us. |
Here's an alternative solution: We could make Drop accept an |
I'm going back and forth on this. Adding an |
Don't let data-{classPrefix} attribute override content option
This adds the `role=tooltip` attr to the tooltip content, and the `aria-describedby` attr on the tooltip target. Requires Drop 1.4.2, due to this change: HubSpot/drop#134
Noticed this while playing around with a solution to HubSpot/tooltip#50. Currently, if the Drop target has a
data-{classPrefix}
attribute, that attribute is used as the Drop content regardless of whetheroptions.content
is set.As far as I can tell, this feature is undocumented, and it's quite confusing. For instance, Tooltip sets
options.content
based ondata-tooltip
on its own, even though thecontent
option is actually ignored by Drop (since Drop'sclassPrefix
is set totooltip
, causing it to take the value ofdata-tooltip
directly as its content).@huoxito This is your code (#107), so what do you think? It seems to me that JavaScript options should take precedence over DOM attributes, but maybe I'm missing some use case. Either way, there should be documentation for this feature. /cc @geekjuice