Tue, Aug 13
@ovasileva this has now been merged, but it is featured flagged and is turned off by default so we'll have to swat deploy to turn it on. The deploy should be pretty easy we only need to set the following config flags:
Mon, Aug 12
Fri, Aug 9
All of your requested changes are now up on http://readers-web-staging.wmflabs.org.
Should the button say "Enable advanced mode"? I think it could be good to reinforce what you're doing by clicking the button.
Thu, Aug 8
Padding is restored
Wed, Aug 7
@alexhollender As mentioned in slack this is now up for design review at readers-web-staging.wmflabs.org, but it's a bit of a pain to test multiple times because it should only show once per browser. A way to get around this is to use a new incognito mode tab in chrome (looks like ios safari might have their own version of that at https://support.apple.com/en-us/HT203036) every time you want to see the drawer. If that’s too much of a pain or doesn’t work, I can add more code to make it easier. To see the drawer now:
Tue, Aug 6
pinging @Jdlrobson since I only code reviewed although I'm having trouble reproducing what alex is seeing at least on my laptop
Thu, Aug 1
@alexhollender Thanks for clarifying!
Wed, Jul 31
@alexhollender sorry for the repeated pings, another question came up
@alexhollender I have three questions for you that are specific to this ticket:
people see the drawer once per device/browser combo, rather than once total
people don't see the drawer at all if another user on the same device/browser combo has already seen it
I've moved this into code review. In my latest patch I implemented the feature using local storage (it was mostly code removal) and pretty easy so I figured it was worth doing even if we end up preferring to use the database.
Tue, Jul 30
As this task has lingered, I want to provide an update on what's happening:
@jcrespo (cc: @phuedx) To implement this feature and given that there is a requirement that the drawer can only be shown once regardless of the device (I will be questioning this requirement in a later post), I think we need to make use of the user_properties table and add a record to that table when the drawer is shown. That record would then serve as a flag to not show the drawer again. However, I'm wondering if the number of records this will add to the database is an acceptable number. It would basically work like this:
Mon, Jul 29
@alexhollender I was thinking for simplicity it would work like this: After the Outreach feature is released and turned on, if the user turns off AMC from their settings page, they would not be eligible to to see the Outreach modal ever again.
@alexhollender @ovasileva I'm thinking we need to make it so that if the user has AMC enabled and then turns it off in their settings, they will also not be eligible to see the outreach modal (even if they haven't seen the outreach modal before). Otherwise, when this feature lands, if people who currently have AMC enabled try to disable it in their settings, they would see the modal appear as soon as they turn off AMC and that would probably be pretty irritating.
Thu, Jul 25
Tue, Jul 23
@alexhollender the task description shows a tablet icon on the drawer; where can I find this icon?
Mon, Jul 22
Jul 18 2019
@Edtadros sorry, I needed to update the QA instructions. You should be able to see the print button if you follow the instructions now
Jul 17 2019
@Edtadros this should be fixed now if you check the page again. Thanks for noticing this and bringing it up!
There are multiple things to the Drawers that contribute to the performance problems. Here is the profile of it on the Obama page when the first reference is clicked using 6x throttle to simulate a mobile device:
Jul 16 2019
Jul 15 2019
Jul 12 2019
The dependency order on these patches is fairly confusing so here are the patches that still need review starting with the order they should be merged in:
Jul 11 2019
Jul 3 2019
Jul 1 2019
Jun 28 2019
Jun 27 2019
Moving this back to To Do as I'm interested to hear Gilles's advice to my initial analysis above
Functionally, doesn't disabling the lazy loading logic in the case of the section toggling mean that lazy-loaded images that might end up inside the viewport as a result of section expansion (top images inside the previously collapsed section) won't load until the user scrolls?
@Gilles Okay, today I carved out time to do some analysis on this. Here were my findings and my interpretation of the findings. Please correct me where I'm wrong!: