-
-
Notifications
You must be signed in to change notification settings - Fork 31
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
Support for dynamically rendered iframe like twitter widget #10
Comments
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 |
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 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 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 I don't even think this needs to be an additional configuration option. The service definition already includes a |
@MoritzLost, there is a You can find 3 custom widget examples in the 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 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). Screen.Recording.2023-07-06.at.15.21.06.movSetting |
I'm not sure why you are setting the div as 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 <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>
|
@orestbida Ok, I tried to put the placeholder inside the element with I'm having trouble implementing this as intended because I don't understand how the Some additional small issues I noticed, let me know if you want me to create separate issues for those:
|
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.
Can you share a demo? For the sake of clarity: you either use
It should be at the bottom right of the div element.
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? |
@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 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 Is there a way around this?
Without
The consent overlay needs to cover that space. I think the correct/intended solution is to use By the way, it's a bit inconvenient that iframemanager uses
It is now, maybe that was related to the other layout methods I tried.
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 |
"ph" (placeholder) refers to the
Mhm, shouldn't it be a transition from black -> iframe rather than a flash? This is what I see: 2023-07-08.05-24-24.mp4
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.
I will need to look this up in more detail, as currently it breaks some of the demo embeds. |
@orestbida I'm seeing a hard transition, for some reason I'm not seeing the placeholder or the transition. Screen.Recording.2023-07-10.at.09.57.48.movWithout the transition and with the callback adding the 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: Screen.Recording.2023-07-10.at.10.14.38.movAny 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
I've just set
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 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');
}
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. |
Technically speaking you can, by checking if the cookie of the mapbox service is set; but its not a pretty solution. Example (inside the <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). |
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 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 – |
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.
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. |
Fair enough – done! #45
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 |
Is there any other problem than the fixed height/width issue? |
Support for dynamically added iframes like:
Ref: orestbida/cookieconsent#195
The text was updated successfully, but these errors were encountered: