Page MenuHomePhabricator

Do banner hiding with mixins
Closed, ResolvedPublic4 Estimated Story Points

Description

Banner hiding is currently done using a small number of javascript files. If we replace this system with “preload” mixin code, an immediate win is that we can prevent injection of banner HTML into the DOM when hide logic is triggered. Other wins worth noting are that we get visibility and code review on tricky hiding logic.

TODO: discussion and rewrite of existing hide logic

Deployment
https://gerrit.wikimedia.org/r/#/c/182107/

https://gerrit.wikimedia.org/r/#/c/181240/

https://gerrit.wikimedia.org/r/182897

https://gerrit.wikimedia.org/r/#/c/181244/

https://gerrit.wikimedia.org/r/#/c/181239/

To convert a banner to use this mixin,

Go to the banner edit page.
Remove the BannerShowHideCount template from the banner content, make a note of your parameter values.
Check the BannerShowHideCount mixin and save the banner.
Banner message fields should appear for the four parameters. Enter the old values into these fields and save again. Note that the parameter names have been changed to protect your sanity.

Event Timeline

atgo created this task.Jan 8 2015, 12:35 AM
atgo assigned this task to awight.
atgo raised the priority of this task from to Medium.
atgo updated the task description. (Show Details)
atgo edited a custom field.
atgo added subscribers: Aklapper, atgo.
atgo set Security to None.Jan 8 2015, 1:49 AM
awight edited a custom field.
awight added a project: Patch-For-Review.

What are the changes to UI/workflow that will happen with this? I'd like to be a part of any decisions around how these changes will effect our users.

The changes should be very minor, the mechanism is almost fully compatible with the "javascript snippet" method of banner dieting.

Admins will have to:

  • Check "Banner diet mixin" for banners that should use this logic.
  • Don't include a javascript snippet to do the same thing!
  • Set banner message variables to control cookie name and diet parameters. Same as before, but possible slight changes to the naming convention.

We should weigh this work against a campaign-level mixin to do the same thing, following T90913. An important part of this decision is the migration plan.

Change 181244 had a related patch set uploaded (by Awight):
Preload JS comes with no baggage

https://gerrit.wikimedia.org/r/181244

Change 181244 merged by jenkins-bot:
Preload JS comes with no baggage

https://gerrit.wikimedia.org/r/181244

Update:

  • All but one of the patches mentioned in this task have merged.
  • The remaining patch has been abandoned. That's the one that actually implements the feature that this task is about. Instead of implementing banner hiding with banner-associated mixins, we'll do it with the new campaign-associated mixins (see T90915).
awight removed awight as the assignee of this task.Mar 12 2015, 10:28 PM
awight moved this task from Doing to Backlog on the Fundraising Sprint Flaming Lips board.

I thought we were done/abandoning this one?

We're doing this a different way. It remains to be decided if we're creating a new card, or renaming this, I think.

Should we close this one as invalid/declined?

awight closed this task as Resolved.Oct 21 2015, 7:18 AM
awight claimed this task.
mmodell removed a subscriber: awight.Jun 22 2017, 9:38 PM