| Jdlrobson | |
| Jun 2 2020, 11:36 PM |
| F32385030: Screen Shot 2020-10-14 at 1.58.02 PM.png | |
| Nov 25 2020, 12:03 AM |
| F32374961: Screenshot 2020-10-06 at 00.45.34.png | |
| Oct 5 2020, 11:47 PM |
| F32374967: Screenshot 2020-10-06 at 00.47.04.png | |
| Oct 5 2020, 11:47 PM |
| F32374963: Screenshot 2020-10-06 at 00.45.47.png | |
| Oct 5 2020, 11:47 PM |
| F32374965: Screenshot 2020-10-06 at 00.46.53.png | |
| Oct 5 2020, 11:47 PM |
| F32374767: Screen Shot 2020-10-05 at 2.28.14 PM.png | |
| Oct 5 2020, 9:28 PM |
| F32374761: Screenshot 2020-10-05 at 22.23.04.png | |
| Oct 5 2020, 9:23 PM |
| F32374512: Screenshot 2020-10-05 at 17.10.27.png | |
| Oct 5 2020, 4:12 PM |
The SkinTemplateOutputPageBeforeExec hook will be soft deprecated in T60137
Preferred hooks are listed in https://www.mediawiki.org/wiki/User:Jdlrobson/Skins_for_extension_developers
In 1.36 the Vector skin will no longer run this hook and depending on migration efforts the hook may be hard deprecated.
Let me know if you have any questions or need any help with this migration. I'll be happy to help you!
| Subject | Repo | Branch | Lines +/- | |
|---|---|---|---|---|
| Replace deprecated hook | mediawiki/extensions/CookieWarning | master | +95 -73 | |
| Replace deprecated hook | mediawiki/extensions/CookieWarning | REL1_35 | +95 -73 |
@Jdlrobson Hi, I'm wondering if you know what hook this extension should migrate? I haven't been able to find a hook that allows you to insert into the header element.
@Jdlrobson thanks for the response! I found a replacement hook for the desktop version onSkinAfterContent, since it'll still shows the banner as if you did the code just under <body> though under MobileFrontend this shows in the footer thus this hook can only be used for the desktop. I've tried SiteNoticeAfterHook & SiteNoticeBeforeHook for mobile, the only issue is it displays where the site notice is, not at the top. Thus it's styles get lost (it's white the background, but because the background is already white it looks weird).
See screenshot linked:
compared to
Is it possible to add a hook to add elements just right under <body>?
I've tried SiteNoticeAfterHook & SiteNoticeBeforeHook for mobile, the only issue is it displays where the site notice is, not at the top. T
This is a much better location for the cookie warning. Banners on Minerva above the header tend to break the UI as Minerva was not designed for anything displayed above the header.
What is your concern with this placement - is this based purely on how it used to work? This seems like a massive improvement to me.
@Jdlrobson I mean I am not sure if this looks nice
Also another concern is what if someone adds a site notice, the cookie notice would be shown under rather than on top, is that fine? The before hook only allowed you to override the site notice or allowed the configured one (not add one before the site notice you configured.).
Seems fine to me - this is consistent with how Minerva is now showing CentralNotice banners, although I would probably add a bottom border to the banner to distinguish it from the bottom with a color that matches the header background.
@Paladox I think 1px is all that's needed here. You'll also want to remove those margins to the left and right. I'm not sure where they are coming from.
Here's a CentralNotice banner for reference which is using border-bottom: 1px solid #c8ccd1;:
@Jdlrobson that looks nice! How was that managed (as in it spread out like that, and the "dismissible" button hidden?
Am not sure why the dismissable button is showing up for you can you share the HTML of that page or even better a test URI?
(I would guess there is a compatibility problem with the DismissableSiteNotice extension.)
Oh, I think your US based. Which we've switched it off for. I'll switch it on so you can see. Please try again. @Jdlrobson
I'm not sure what's going on here, but it seems that the CookieWArning extension is being wrapped in a DismissableSiteNotice banner. DismissableSiteNotice is not listed as a dependency of that extension. Can you disable that extension and confirm it works without that enabled?
I suspect there is a bug in that extension. When I apply the following rules CookieWarning looks fine:
`
.mw-cookiewarning-container { border-bottom: 1px solid #c8ccd1; }
.sitedir-ltr .mw-dismissable-notice-body { margin: 0; }Please raise a bug for DismissableSiteNotice. We're currently moving this code into core (T259903) so should be able to fix styling in Minerva as part of that move.
Change 631814 had a related patch set uploaded (by Paladox; owner: Paladox):
[mediawiki/extensions/CookieWarning@master] Replace deprecated hook
@Jdlrobson Is what it looks like now with (https://gerrit.wikimedia.org/r/631814) , which seems fine for now.
and
Doing https://gerrit.wikimedia.org/r/632335 should make it look like:
and
Change 631814 merged by jenkins-bot:
[mediawiki/extensions/CookieWarning@master] Replace deprecated hook
Suggested follow up: I would suggest reviewing the absolutely positioned banner given the upcoming changes to Vector detailed here:
https://mediawiki.org/wiki/Desktop_improvements
I testet the new Master branch on my wiki. Now it works with the Vector skin. but now my mobile view with MinervaNeue doesn't work anymore. With the old REL1_35 branch it's just the other way round. I hope you could also fix that.
Change 643961 had a related patch set uploaded (by Paladox; owner: Paladox):
[mediawiki/extensions/CookieWarning@REL1_35] Replace deprecated hook
Change 643961 merged by jenkins-bot:
[mediawiki/extensions/CookieWarning@REL1_35] Replace deprecated hook