Skip to content
This repository has been archived by the owner on Feb 1, 2022. It is now read-only.

Add warning if sanitize is set to false for tooltips and popovers #444

Closed
wants to merge 1 commit into from
Closed

Add warning if sanitize is set to false for tooltips and popovers #444

wants to merge 1 commit into from

Conversation

mcmxcdev
Copy link

Closes #434.

@Johann-S
Copy link
Member

Hi @mhatvan,

Thanks for the PR, unfortunately it's not a good way to check that because:

image

We have to check for Tooltips and Popovers the internal config to see if sanitize is equal to false

@mcmxcdev
Copy link
Author

@Johann-S aww, too bad, I must have overread this note in the options when I was reading the documentation.
Where do I find the internal config so that I can implement it?

@Johann-S
Copy link
Member

you should select all the tooltips and popovers by doing that: $('[data-toggle="tooltip"]', [data-toggle="popover"]) and you'll be able to do : $(tooltip).data('bs.tooltip') which will return to you an instance of our Tooltip class a like that you'll find the config attribute 😉

@mcmxcdev
Copy link
Author

mcmxcdev commented Jun 27, 2019

@Johann-S Can't get it working according to your explanation.

$('[data-toggle="tooltip"]').data('bs.tooltip') returns undefined.

In a bootstrap project in the browser I can do $('[data-toggle="tooltip"]').tooltip(), then access the default options set by bootstrap through $('[data-toggle="tooltip"]').data('bs.tooltip').options.

When running bootlint.js with yarn test with the same code, I receive $(...).tooltip is not a function

@Johann-S
Copy link
Member

Johann-S commented Jun 30, 2019

if you have multiple tooltip triggers this jQuery cal $('[data-toggle="tooltip"]') will return an array, so you have to do:

$.each($('[data-toggle="tooltip"]'), function ($tooltip) {
	$tooltip.data('bs.tooltip') // => return a Tooltip instance
})

@mcmxcdev
Copy link
Author

mcmxcdev commented Sep 2, 2019

Tried again, can't get it to work, maybe someone else wants to take over this PR.

@Johann-S
Copy link
Member

Johann-S commented Sep 3, 2019

Hi @mhatvan,

You should try that:

$('[data-toggle="tooltip"], [data-toggle="popover"]').each(function () {
	var dataKey = $(this).attr('data-toggle') === 'tooltip' ? 'bs.tooltip' : 'bs.popover'
	var obj = $(this).data(dataKey)
	if (!obj.config.sanitize) {
	  // warn here
	}
})

Do not hesitate to tell me where you're stuck 😉

@mcmxcdev
Copy link
Author

Your code snippet looks solid, but doesn't work either -> $.each is not a function

@Johann-S
Copy link
Member

Oh yes you're right @mhatvan, I updated my code, it should be fine now 😄

@mcmxcdev
Copy link
Author

mcmxcdev commented Sep 12, 2019

I receive the correct value for $(this).attr('data-toggle') but $(this).data(dataKey) returns undefined.

$(this).data() only holds e.g: { toggle: 'popover', sanitize: false, content: 'This is a popover!' }

So how to initialize the popover correctly?

Also, since it is in an .each loop, the reporter would spit out the same error message for every non-passing element, is this correct behavior?

@Johann-S
Copy link
Member

It's not your job to initialize the popover/tooltip, you have to check if an initialized Popover/Tooltip disabled the sanitize option.

Also, since it is in an .each loop, the reporter would spit out the same error message for every non-passing element, is this correct behavior?

Yup it's seems correct to me 🤔 maybe @Herst have some feedbacks

@Herst
Copy link
Collaborator

Herst commented Sep 15, 2019

There should be only a single call to reporter() but with an array of elements as the second argument (one per offence). Your original code was correct in this regard.

@mcmxcdev
Copy link
Author

I tried again getting this to work, in the browser it works fine, in bootlint.js not.

var obj = $(this).data(dataKey) doesn't work because there is no bs.tooltip property in $(this).data() that can be accessed, therefore resuting in Cannot read property 'config' of undefined.

@Johann-S
Copy link
Member

Hi @mhatvan if you're unable to retrieve something from .data(dataKey) it means there is no plugin initialized, so you can skip this one

@mcmxcdev
Copy link
Author

Well, that's what I receive for all tooltips and popovers, so I can't access any options and can't implement the reporter to report on anything.

@Johann-S
Copy link
Member

So you have to initialized some of them before running your tests to simulate a real use case 😉

@mcmxcdev mcmxcdev closed this Dec 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add warning if sanitize is set to false
3 participants