Open melker opened 2 years ago
Is there already a testable solution for this to provide feedback?
@MaluNoPeleke, it is possible but not ideal.
Unlike traditional iframes, twitter widgets handle their width/height dynamically, which doesn't fit well with iframemanager. You need to specify a fixed height/width to avoid content jumping when the iframe is loaded.
+1 for this feature request for me. We have a lot of use-cases for external services that aren't loaded as iframes, but as custom scripts. For example, we often include custom Google Maps oder Mapbox maps using Leaflet or Mapbox GL. In this case, the maps are created using those JS frameworks, not as an iframe. But loading the bitmap or vector tiles requires consent, so we need a placeholder with the option to consent.
In my opinion, this feature can be implemented in a really basic way, it doesn't need a lot of functionality:
Basically, all I would need to consider this feature complete is the ability to tell iframemanager to insert the consent placeholder for a given category in a specific element. Currently this is not possible, so I would have to build my own interface to match iframemanager's interface for the use-cases described above.
As a reference, take klaro's contextual consent feature, which works in a similar way: https://klaro.org/docs/tutorials/contextual_consent
Basically, all I would need to consider this feature complete is the ability to tell iframemanager to insert the consent placeholder for a given category in a specific element. Currently this is not possible, so I would have to build my own interface to match iframemanager's interface for the use-cases described above.
Mhm, I'm not sure I understand. If you have — say — 2 twitter embeds, why would you want to have 2 different consent placeholders even though they belong to the same service?
@orestbida Sorry, maybe I didn't explain it well. I don't want to have two different placeholders. I want to use iframemanager as a placeholder for external content that isn't delivered in the form of an iframe.
For example, I am working on a map component that is powered by Mapbox and initialized through JavaScript using mapbox-gl
. In this case, there's no iframe involved, since the map is initialized by my custom JavaScript code. Since loading Mapbox tiles requires consent, I need a placeholder with consent buttons. However, after consent is given, I don't need an actual <iframe>
on the page, since the map is initialized by my JS callback.
I can actually get really close to what I want with iframemanager:
const initializeMap = () => {
// JS code that initializes the map using mapbox-gl
}
im.run({
onChange({ changedServices, eventSource }) {
if (eventSource.action == 'accept' && eventSource.service == 'mapbox') {
initializeMap();
}
},
})
The problem is that iframemanager always inserts an <iframe>
into the page and forces it to overlay the existing content. I can solve this by hiding the iframe (or wrapper element) using JS, but that's not a very clean solution.
So to summarize, I want to use the placeholder provided by iframemanager, but I don't need iframemanager to actually insert an iframe in some cases.
More broadly: We're trying to use cookieconsent
and iframemanager
as a general, GDPR-compliant consent manager solution. Having placeholders for external content is very important – but not every external content is loaded in the form of an iframe. Being able to use the same placeholders for external content regardless of delivery method (iframe or custom JS code) would solve this. This would require an option or configuration that tells iframemanager not to insert an iframe for specific placeholders.
I don't even think this needs to be an additional configuration option. The service definition already includes a embedUrl
config – maybe just allow users to leave this empty and skip inserting the iframe is this is the case? Or if you don't like that approach, maybe this can be controlled through another data-*
attribute on the placeholder element, data-no-iframe
or something like that?
@MoritzLost, there is a custom widgets
section which describes how to do this. You use the onAccept
and onReject
methods to define how the markup is generated and destroyed.
You can find 3 custom widget examples in the /demo
dir:
Mapbox should be very similar to leaflet.
Edit: here's also a demo which shows how to connect iframemanager <-> cookieconsent
@orestbida Thank you, I totally missed that! Sorry for the confusion. Now it's working fine, I'm triggering a CustomEvent
in the global onChange
callback (and the onAccept
callback as well) and using that to initialize my map once consent is given. Then I set div.hidden = true
in the onAccept
callback.
This works and now the map is displayed correctly!
As a sidenote, I was seeing an issue where I get a flash of black before the overlay is hidden (see screen recording below).
https://github.com/orestbida/iframemanager/assets/10146880/acec9c4d-cbfb-48fa-a9f6-0474be341915
Setting div.hidden = true
fixed that. I think that would be a sensible solution to put into iframemanager itself. That would also be good for accessibility, since hidden
removes the placeholder from the accessibility tree completely – right now the placeholder is just hidden using opacity
and visibility
. Should I create a separate issue for that?
I'm not sure why you are setting the div as hidden
, when the generated widget is appended inside the div.
Perhaps this issue is specific to Mapbox, as I don't see it in the above 3 mentioned custom widgets.
Perhaps I'm lucky, but I don't see the mentioned issue in this basic example (tested in Chrome and Firefox):
<style>
.mapbox {
height: 100%;
width: 100%;
}
</style>
<div data-service="mapbox" data-autoscale>
<div data-placeholder>
<div id="map" class="mapbox"></div>
</div>
</div>
{
/**
* @param {HTMLDivElement} div
*/
onAccept: async (div) => {
await loadScript('https://api.mapbox.com/mapbox-gl-js/v2.9.1/mapbox-gl.js');
await im.childExists({childProperty: 'mapboxgl'});
mapboxgl.accessToken = 'YOUR_TOKEN';
const map = new mapboxgl.Map({
container: 'map',
style: 'mapbox://styles/mapbox/streets-v11'
});
await map.once('load');
// Manually show placeholder
div.classList.add('show-ph');
},
/**
* @param {HTMLDivElement} serviceDiv
*/
onReject: (iframe, serviceDiv) => {
serviceDiv.lastElementChild.firstElementChild.remove();
},
languages: {
// ...
}
}
@orestbida The map is not rendered inside the placeholder in my case. The code that initializes the map is in a separate file, I don't want to have that inside the iframemanager callback and I don't want to couple the map code to iframemanager. The map code just listens to the CustomEvent
emitted inside the onAccept
callback so it knows it's ok to load the map. That's why I have separate elements for the placeholder and the mount point for the map:
<div class="location-map">
<div class="location-map__placeholder" data-service="mapbox" data-widget></div>
<div class="location-map__mount" data-map-mount></div>
</div>
location-map
is used as a wrapper and provides the map dimensions. The placeholder and map mount are both set to fill the container using position: absolute
. Once consent is given, the placeholder is hidden and the map is initialized inside the data-map-mount
element. Maybe setting the placeholder to position: absolute
is causing the flash of black background? I can't use fixed width/height because the dimensions are dynamic and based on the viewport, and the placeholder needs to occupy the same space the rendered map will.
@orestbida Ok, I tried to put the placeholder inside the element with data-widget="mapbox"
, but it doesn't work. The black placeholder doesn't go away, it stays there after interacting with one of the buttons.
I'm having trouble implementing this as intended because I don't understand how the setIframe
method is supposed to be used. What does it do exactly, and what element should I pass it if I'm not using an iframe? Can I skip calling it at all?
Some additional small issues I noticed, let me know if you want me to create separate issues for those:
data-autoscale
option doesn't work well because it uses height: auto
instead of height: 100%
. This should probably be heigth: 100%
? Or how is this intended to work?The plugin was never designed to handle these types of custom use-cases that you're trying to achieve (data-service div wrapped in your own div, and with your custom logic handling).
You need to use a specific structure for this to work as intended. I posted the full example with mapbox in a comment above and I don't face any of the mentioned issues.
The data-autoscale option doesn't work well because it uses height: auto ...
Can you share a demo?
For the sake of clarity: you either use data-autoscale
or data-widget
, not both.
I noticed the placeholder includes a tiny spinning element at the top right ...
It should be at the bottom right of the div element.
It's invisible by default, but is briefly visible during the transition after clicking one of the buttons.
Yes, to show that "something" is loading. If the widget/iframe loads incredibly quickly, then the loader might not show at all or show for an extremely brief moment, which is normal. Why do you want it removed?
The plugin was never designed to handle these types of custom use-cases that you're trying to achieve (data-service div wrapped in your own div, and with your custom logic handling).
@orestbida Fair enough. I've given it another try – now I'm following the recommended structure and it's working as intended. Some details below.
<div class="location-map">
<div class="location-map__placeholder" data-service="mapbox" data-widget>
<div class="location-map__mount" data-placeholder data-map-mount></div>
</div>
</div>
onAccept: div => {
initializeMap();
div.classList.add('show-ph');
},
Sidenote, why is the class called show-ph
? Isn't the consent overlay the placeholder, and the class reveals the actual content beneath the placeholder?
However, now I'm definitely seeing the issue with the flash of black. I think this is the expected behaviour. All the examples in the demo show the same behaviour. If you set everything to Don't ask again
and then reload the page, you still see the overlay briefly before the actual content has loaded. That's because all the examples assume that the content will be loaded asynchronously, so the overlay will be visible for at least a couple of seconds. Since we initialize the map synchronously, the overlay only briefly flashes during the page-load, then is immediately hidden when my callback function runs.
Is there a way around this?
For the sake of clarity: you either use data-autoscale or data-widget, not both.
Without data-widget
, iframemanager forces a specific aspect ratio using the padding-top
method on the ::before
method. But my map doesn't have a fixed aspect ratio – the width is always 100% of the viewport, but the height is set dynamically based on the viewport height:
height: min(80vh, 32rem);
The consent overlay needs to cover that space. I think the correct/intended solution is to use data-widget
and then set width: 100%
and height: 100%
?
By the way, it's a bit inconvenient that iframemanager uses all: unset
, this forces me to use !important
or use a more specific selector everywhere I need to overwrite something (like in this case).
It should be at the bottom right of the div element.
It is now, maybe that was related to the other layout methods I tried.
Yes, to show that "something" is loading. If the widget/iframe loads incredibly quickly, then the loader might not show at all or show for an extremely brief moment, which is normal. Why do you want it removed?
Got it, thanks – in my current use-case, I'm hiding the consent overlay immediately. The map is displayed pretty much immediately in our case because our JS code that initialies the map is already loaded at this point (and it contains mapboxgl
as it's installed via NPM and compiled together with our JS). But the loading indicator doesn't hurt in any case.
Sidenote, why is the class called show-ph? Isn't the consent overlay the placeholder, and the class reveals the actual content beneath the placeholder?
"ph" (placeholder) refers to the div
with the data-placeholder
attribute, although I also am not fond of this name.
However, now I'm definitely seeing the issue with the flash of black.
Mhm, shouldn't it be a transition from black -> iframe rather than a flash? This is what I see:
https://github.com/orestbida/iframemanager/assets/34487268/76787f99-60e2-4040-b230-31eab13e7bee
By the way, it's a bit inconvenient that iframemanager uses all: unset, this forces me to use !important or use a more specific selector everywhere I need to overwrite something (like in this case).
This is (sadly) needed, as there are badly coded websites, that apply global styles that might ruin the layout/look of the plugins generated markup.
The consent overlay needs to cover that space. I think the correct/intended solution is to use data-widget and then set width: 100% and height: 100%?
I will need to look this up in more detail, as currently it breaks some of the demo embeds.
Mhm, shouldn't it be a transition from black -> iframe rather than a flash? This is what I see:
@orestbida I'm seeing a hard transition, for some reason I'm not seeing the placeholder or the transition.
https://github.com/orestbida/iframemanager/assets/10146880/0db5926a-ef15-4145-94a6-cc5092e15442
Without the transition and with the callback adding the ph-show
class immediately, it's more of a flash of black.
Just to rule out any issues with my implementation, I tried switching to a fixed width/height and making my callback asynchronous with some delay:
onAccept: div => new Promise(resolve => {
triggerConsentManagerEvent('mapbox', true);
setTimeout(() => resolve(div.classList.add('show-ph')), 2000);
}),
But the loading indicator is still not showing, though I do get a short transition now:
https://github.com/orestbida/iframemanager/assets/10146880/012dcacc-4118-4236-aabb-1fdeee20a40f
Any ideas why? What is triggering the loading indicator and the transition? The only difference between the demos and my implementation is that I'm not using im.childExists
, can that have something to do with it?
I will need to look this up in more detail, as currently it breaks some of the demo embeds.
I've just set width: 100% !important
and height: 100% !important
on the placeholder, instead of setting a fixed width/height as in the demos – shouldn't make that much of a difference? Anyway, fixed width & height rarely makes sense for responsive website, so this should be addressed if it is an issue.
"ph" (placeholder) refers to the div with the data-placeholder attribute, although I also am not fond of this name.
Agreed – in my mind the consent overlay is the 'placeholder', so in the callback I'm hiding the placeholder and showing the actual content.
@orestbida Is it possible to somehow skip displaying the black placeholder if consent has already been given? I know my map will load almost instantly, so it will always look like an error if I get a briefly visible black overlay.
@MoritzLost, you need to make sure that the mapbox widget is fully loaded before toggling the show-ph
class. Also the triggerConsentManagerEvent
function needs to be async
or return something promise-like.
The simplest way is to use the async/await syntax:
async function triggerConsentManagerEvent(service, enable) {
// ...
const map = new mapboxgl.Map({...});
// important
await map.once('load');
}
onAccept: async (div) => {
await triggerConsentManagerEvent('mapbox', true);
div.classList.add('show-ph');
}
The only difference between the demos and my implementation is that I'm not using im.childExists, can that have something to do with it?
Nah, this method is a 'safeguard', used to make sure that an object or dom element actually exists, before accessing it. When dynamically loading a script, it may occur that although the script is fully loaded (downloaded), it might not yet be fully evaluated/executed. childExists
does basically this: check every x ms, until it (object/dom element) exists.
Is it possible to somehow skip displaying the black placeholder if consent has already been given?
Technically speaking you can, by checking if the cookie of the mapbox service is set; but its not a pretty solution.
Example (inside the head
section):
<script>
// check if "im_mapbox" cookie is set
if(document.cookie.includes("im_mapbox="))
document.documentElement.classList.add("im-mapbox");
</script>
<style>
.im-mapbox div[data-service="mapbox"]{
--im-bg: transparent;
}
</style>
You also must handle the case when the service is rejected (remove the "im_mapbox" class).
you need to make sure that the mapbox widget is fully loaded before toggling the show-ph class.
Ok, I think I found the issue. My map takes about ~200ms to fully initialize and load all tiles, and the transition is shorter than that. Thanks for the suggestions.
I could adjust my code to wait until all tiles are loaded before adding the ph-show
class – but from a UX perspective, that is really a step backwards. I would much rather show the underlying map immediately. On a slower connection, the user will be able to see the map tiles as they are loaded, this is a much better indicator of the map loading than the placeholder. So the black overlay will be visible only very briefly, which looks buggy to the end-user.
I feel like this might be a common use-case – having a placeholder for content that will be visible immediately, so the placeholder shouldn't show up at all on consequent page loads. Would it be possible to add a configuration option for this, or a CSS property to customize this? That is, have a way to tell iframemanager not to show the overlay at all and instead reveal the underlying contents immediately if consent has already been given previously.
It was really difficult to debug this because of the obfuscated class names – .c-an .cll .c-la-b
etc … what's going on there?
This lib. was designed to be a more "elegant" approach to the usual "load->flash iframe" implementation, which is in contrast with what you're asking. IMO the current approach is the best option to handle all use cases, even though it makes the loading of external resources feel more "sluggish".
With that said, I will consider adding an attribute (and javascript option) to handle this particular preference/use-case. Please open a new issue so that we can better track this feature.
It was really difficult to debug this because of the obfuscated class names – .c-an .cll .c-la-b etc … what's going on there?
Both iframemanager v1.x.x and cookieconsent v2.x.x use short/cryptic classes, for the sake of a lighter plugin. Although they are gibberish to the users, they have a meaning for me as the author of the plugin.
With that said, I will consider adding an attribute (and javascript option) to handle this particular preference/use-case. Please open a new issue so that we can better track this feature.
Fair enough – done! #45
Both iframemanager v1.x.x and cookieconsent v2.x.x use short/cryptic classes, for the sake of a lighter plugin. Although they are gibberish to the users, they have a meaning for me as the author of the plugin.
I very strongly disagree with this. The library has <500 lines of CSS, so a couple dozen selectors at best. Using readable class names would add a couple of kilobytes at best. And those can be compressed very well using DEFLATE, so we're talking miniscule amounts of bandwidth. For those few kilobytes the library is greatly improved:
My suggestion would be to use readable class names, ideally scoped to a namespace (like .im-*
). To save on bytes, you can in turn drop all the div
element selectors – they're not necessary at all.
@MaluNoPeleke, it is possible but not ideal.
Unlike traditional iframes, twitter widgets handle their width/height dynamically, which doesn't fit well with iframemanager. You need to specify a fixed height/width to avoid content jumping when the iframe is loaded.
Is there any other problem than the fixed height/width issue?
Support for dynamically added iframes like:
Ref: https://github.com/orestbida/cookieconsent/issues/195