Code Tour: 2019-04-29 to 2019-05-29

[roadrunnertwice]

Looks like it was kind of a busy month, so... time for a code tour! (First ever from yours truly. Content note: includes some explanatory GIFs.)

Most of these changes are scheduled for Soon, but a few of the smaller ones are already live on the site. Themes this month: Improvements to mobile, code cleanups, fixes for bugs that snuck in during recent work, and the ongoing march of TT-ification.

Issue 844: Use a more logical HTML structure for the control strip (pull request)
Issue 833: Control strip: take up the entire width of the screen if possible
Issue 835: Control Strip: Don't disallow line wrap on smaller screens
Issue 858: Control Strip login form goes onto two lines
Issue 1335: can we remove nowrap from navstrip?
Category: Control Strip Redesign, Telephone Love
Patch by: nfagerlund
Description: On most user-styled journal pages, there's an optional bar at the top called the control strip — it's been around forever, and its layout behavior has changed a few times to support mobile browsers and the like. But since the function that generated it was extremely gnarly and hard to edit, those updates were CSS hacks that left the existing HTML alone. That HTML was starting to look pretty hairy, and some minor bugs were piling up that should have been easy to fix but (because the actual markup was finely minced and distributed across several hundred lines of Perl) weren't. So this was a two-part fix:

1. Extract the existing controlstrip HTML into a template, so future edits will be easy.
2. Replace the huge Mutant Table Thing with some basic flexbox divs, to make it sleeker in general and fix a gang of annoying visual glitches.

The first part is already live, but is unnoticeable. The second part goes live Soon, and is... mostly unnoticeable.

Issue 2417: TT-ified /logout controller breaks control strip "log out" button (pull request)
Category: Control Strip Redesign
Patch by: nfagerlund
Description: The reason the first part of the control strip update went live early is that PR 2230 (see previous code tour) broke the control strip's logout button, and it was easier to fix that once the markup was all in one place. A nice concise demonstration of why we're bothering with all this template stuff in the first place.

Issue 827: Don't apply page margins to nav strip
Category: Styles, Control Strip Redesign
Patch by: nfagerlund
Description: Most of the journal styles descended from Tabula Rasa will let you set margins for the content of the whole page. Those margins weren't supposed to affect the control strip at all... but they totally did. Now they don't!

If you have a custom style with that issue and want to fix it: try setting your margins on the #canvas div (which contains all page content except the control strip) instead of on the whole body.

Issue 1568: [#1568] Use a named constant for username maxlength (pull request)
Category: Getting the Coding Up To Plum
Patch by: alexwlchan
Description: Quick, what was the max length for a username? TRICK QUESTION, different places in the code used different max values! Now they're unified.

Issue 1852: add absolute time to inbox timestamps (pull request)
Category: Reducing confusion
Patch by: alexwlchan
Description: Notifications in your inbox traditionally show a relative date like "6 hours ago", which is conveniently terse 80% of the time and entirely useless 20% of the time. Now inbox notifications also show their absolute dates. Like this:

Issue 1944: [#1944] Include the comment URL on screening/unscreening confirmation page (pull request)
Category: Reducing confusion
Patch by: alexwlchan
Description: You can screen or unscreen comments via links in the notification emails, which is very convenient! But the confirmation page that opens in your browser didn't say which comment you'd modified, which was less convenient (especially if you mis-clicked one). Now that page has a link to the affected comment.

Issue 2195: beta update page broken in portrait mode in Chrome on Android
Category: Dep't of Positive Vibes Blast Radius
Patch by: rshatch
Description: This issue was fixed when rshatch removed fastclick.js from the site (in order to resolve several other issues). See also previous code tour.

Issue 2254: Consider removing LJ::User functions and methods unused since at least August 2009 (pull request)
Category: Code machete
Patch by: alexwlchan
Description: This code was doing literally nothing. Now it's doing even less. 🔪

Issue 2275: Useless variable in LJ::Protocol lines 1494&1496 (pull request)
Category: Code machete
Patch by: alexwlchan
Description: More code that did literally nothing.

Issue 2294: remove code referencing ADV_PER_PAGE
Category: Dep't of Positive Vibes Blast Radius
Patch by: zorkian
Description: This was unused code, but it got blown away in the process of fixing something else before anyone could go after it specifically. I'm only keeping it in the tour so you can bask in the fuzzy feeling of being on a site where a primary maintainer will post an issue comment that reads, in full, "lol ads".

Issue 2331: Upgrade to recaptcha 2 (pull request)
Category: Bug fix
Patch by: rshatch
Description: The ReCAPTCHA service stopped serving the old version of its API, so... we had to stop using that old version.

Issue 2410: "Less-quick reply" form blows out the layout on mobile (pull request)
Category: Telephone Love
Patch by: nfagerlund
Description: DW has two different versions of the quick-reply form:

• A minimal one, which appears on pages with a whole bunch of entries (reading page, recent entries, etc.).
• One with a few more options, for single-entry-with-comments pages.

The minimal quick-reply behaved fine on mobile, but the other one still used a crusty table-based layout that was just a mess on telephones. Now the bigger quick reply is the same as the smaller one (plus a few buttons), and both versions of the form are a little more responsive.

Issue 2415: Beta "create entry" page has outrageous compounding blowout on mobile (pull request)
Category: Telephone Love, Slapstick
Patch by: nfagerlund
Description: If you belong to any communities, the beta new-entry page has a drop-down to choose which journal/community you're posting to. And at mobile widths, some baffling CSS and JavaScript interactions made that drop-down overflow sideways. By a lot. And every time you changed its value it would overflow even more. This is best experienced in gif form:

So, that's fixed.

Issue 2421: Most form fields have "Dramatic Woodchuck" zoom on mobile
Category: Telephone Love, Slapstick
Patch by: nfagerlund
Description: When you activate a form field whose font size is smaller than 16px, mobile browsers will aggressively zoom the viewport, like in that video with the "dun-da-DUNNNN" woodchuck:

This behavior is meant for forms that are too small to use on mobile, and it's super annoying if it happens on an otherwise mobile-ready page. But now it's fixed on the beta new-entry page and the quick-reply forms.

We're keeping the issue open to collect other form fields that have the same problem. If you notice any that aren't already listed, please add them to the issue!

Issue 2420: Goldleaf layout: sideways slop due to footer overflow
Category: Styles
Patch by: nfagerlund
Description: With its default settings, the Goldleaf layout always had a tiny bit of sideways scroll in most browsers, because CSS box-sizing can be a little squirrelly.

Issue 2422: Some tweaks to fix XML issues and bad MySQL (pull request)
Category: Content Importing
Patch by: rshatch
Description: There are a few rare but persistent bugs with importing journals and communities from LiveJournal, and they're real hard to tell apart because the errors DW gets back from LJ are Not Informative. This PR fixes one of the really nasty problems, and no one is 100% positive why it works. Do not breathe on it.

Issue 2423: Keys on reference is experimental, so we should properly dereference them (pull request)
Category: Getting the Coding Up To Plum
Patch by: rshatch
Description: Some Perl inside-baseball. Hashes and hashrefs are very different things that can be made to act like the same things, and sometimes the rules for how you distinguish them go through an awkward period of flux.

Issue 2425: Discord field on profile is over-escaped
Category: Profile
Patch by: zorkian
Description: Remember last code tour, when we added Discord to profiles? Unlike nearly every other service, Discord usernames have # characters (hash mark? pound sign? octothorpe? buddy, I have spent way too many hours of my life talking about this character, don't @ me), and the profile code was doing the wrong thing with those because it didn't expect them. Now it's fixed.

Issue 2430: Fix variable substitutions in error messages (pull request)
Issue 2463: [[comm]] variable failing to render correctly in permissions error message
Category: Bug fix, TT Conversion
Patch by: kareila
Description: Some new behavior for TT-based pages required changing the arguments for the function that prints error messages. That broke some things that relied on the old order of arguments, but now they're all speaking the same language.

Issue 2431: API release hotfixes (pull request)
Category: API
Patch by: rshatch
Description: Fixes for some small bugs in the new API (see previous code tour):

• Fix SQL statement for deleting API keys
• Actually return picid as an integer, like we claim we do
• Capture all path parameters, not just the first one
• Loosen JSON validation for incoming data, because Perl doesn't care

Issue 2433: Cleaner positioning code for Tectonic header elements, via flexbox (pull request)
Category: Styles
Patch by: rshatch
Description: An improvement to a previous fix.

Issue 2434: Don't insert script tags that invoke jquery at random points in the page content (pull request)
Category: Getting the Coding Up To Plum
Patch by: nfagerlund
Description: A pre-emptive fix for a bug I haven't caused yet. Some pages were inserting JavaScript snippets that depend on the jQuery library into the body text, at unpredictable places — this worked fine today because of a technical coincidence, but there was no guarantee it would keep working.

Issue 2444: dreamwidth.org/mobile should have a "log out" link when logged in, probably (pull request)
Category: Dep't of There Are Four Lights
Patch by: alexwlchan
Description: On the /mobile page (the name is a bit outdated, but it has a `?format=light` index-only version of the reading page), the log out link was claiming to be a log in link.

Issue 2457: layersource no longer works to view public layers (pull request)
Category: TT Conversion, Bug fix
Patch by: rshatch
Description: Converting the "Advanced Customization" pages to TT (see previous code tour) broke the feature for viewing the source of the built-in S2 layers. Now it's fixed.

Issue 2466: Convert /editprivacy to Foundation and fix form errors. (pull request)
Category: TT Conversion
Patch by: rshatch
Description: Converting an old BML page (which had weird bugs) to a more maintainable TT page. You know the beat of this song by now, right? This time it's the page for changing the privacy level of a bunch of posts at once.

Issue 2467: Run perltidy on everything. (pull request)
Category: Automation, Quality of Dev-Life
Patch by: zorkian
Description: When you're working with a bunch of other people on some code, you have a couple of options:

• Have a lot of tedious, identical conversations about where to put spaces and how to align brackets.
• Give up and let the code style be a mess.
• Delegate all the pedantry to a robot.

We have now chosen option number three. 👍🏼

31 total issues resolved
Contributors: alexwlchan, kareila, momijizukamori, nfagerlund, rshatch, zorkian

Comments

[azurelunatic]

Yay!

[sporky_rat]

for the love of sweet baby torgo and jdn do not breathe on the importer

[cesy]

I can write things and reply to things on mobile and it won't do the weird zoom thing, yay!

[jazzfish]

Yay fixes!

hash mark? pound sign? octothorpe?

Shibboleth.

[turlough]

Issue 1852: add absolute time to inbox timestamps

Hooray!!!

[casimirian]

Woo!

[brainwane]

Thank you all for the improvements and thank you roadrunnertwice for the code tour!

In particular I'd like to thank y'all for add absolute time to inbox timestamps and Upgrade to recaptcha 2, which will specifically help me, and for the continuing progress on the new API!

[juliet316]

Category: Dep't of There Are Four Lights

ROFLMAO!!!