AprilSylph
feat(Tweaks): "Move post permalinks to the attributed blogname"
Description
- resolves #2007
- implements https://github.com/AprilSylph/XKit-Rewritten/issues/2007#issuecomment-3599875828
And the lady of the castle saw no need for any other permalinks, for the ones she had fashioned herself were perfect.
Testing steps
- Load the modified addon
- Log into a Tumblr tab with an account that has access to the
postChromeBodyNavigationEventsRedesignfeature - Enable Tweaks → "Move post permalinks to the attributed blogname"
- Expected result: Hovering a post header's blank space does nothing
- Expected result: Clicking a post header's blank space does nothing
- Expected result: Hovering a trail item header's blank space does nothing
- Expected result: Clicking a trail item header's blank space does nothing
- Double-click a word in a post
- Expected result: The word is selected
- Expected result: Peepr is not opened
- Double-click and drag to select multiple words in a post
- Expected result: The words are selected
- Expected result: Peepr is not opened
- Click a post header's blog attribution
- Expected result: The post is opened in Peepr with no redirects
- Click a trail item header's blog attribution
- Expected result: If the permalink for the added content has a slug, Peepr is opened with one redirect (slugless → slugful URL)
- Expected result: If the permalink for the added content has no slug, Peepr is opened with no redirects
- Disable Tweaks → "Move post permalinks to the attributed blogname"
- Expected result: Hovering a post header's blank space shows the anchor
hrefin your browser - Expected result: Hovering a post header's blank space makes its background change colour
- Expected result: Clicking a post header's blank space opens that post in Peepr
- Expected result: Hovering a trail item header's blank space shows the anchor
hrefin your browser - Expected result: Hovering a trail item header's blank space makes its background change colour
- Expected result: Clicking a trail item header's blank space opens that post in Peepr
- Expected result: Clicking any other blank space in the post body opens that post or trail item in Peepr
- Expected result: Clicking any non-link text in the post body opens that post or trail item in Peepr
- Expected result: If there were any audio players on the page at the time of disabling the tweak, those are still usable without opening the post in Peepr
- Expected result: Hovering a post header's blank space shows the anchor
Not sure if you saw my previous comment about this: any alternative feature name ideas? I don't think it's obvious what an "attributed blogname" is/that this is the feature one is looking for if they were looking for the prior name/functionality.
I was thinking that this option is complex enough to be promoted to its own feature—"Classic Permalinks"—but we've never done that before, so there's no existing infrastructure to do that without forcing every current user to enable it again manually.
For now, maybe "Make blog names link to their associated posts" would work...? I was thinking something along those lines for the standalone feature description.
I'll admit that I don't view the new feature as an expanded version of the old feature, so in my mind, users having to re-enable isn't necessarily bad? This admittedly doesn't work great when the original feature is subtle (a user might not notice it not working for a while, and also when they do they absolutely will just assume xkit is broken rather than clicking the preference button to check... okay, yeah, no, it sucks.)
Not a huge fan of a bunch of users thinking Tumblr reverted part of the a/b, ended the a/b, or (if they didn't have the a/b) getting the feature rollout and being confused about the thing their friends are reporting not happening to them... eh, all options have some downsides, what can you do ¯\_(ツ)_/¯ Anyway:
"Make blog names link to their associated posts"
Well, that just describes the original feature, not the new functionality, right?
I had originally mentioned "Move post permalinks to the blog link in the post/trail header," which is presumably too long. The original is "Restore links to individual posts in the post header"—I guess we can't necessarily do "Move links to individual posts to the post header"/"Move post permalinks to the post header" because that now sounds like it's linking the whole post header, which this PR actively removes. "Move post links to their headers' blog names"—eugh, no. (Aside: I feel like part of why that sounds like it doesn't make sense is because it accurately describes a feature that doesn't make sense—imo, design-wise, the header is the correct place for the permalink—but that's neither here nor there.)
"I wonder if we can steal some language from the changes post about the feature! Those are written by people who are paid to use words to describe UI changes."
Some of you will see a refreshed post design on Tumblr that makes posts easier to read and lets you tap anywhere on a post to open its page and join the conversation, as part of an experiment to improve how you browse and interact with posts.
"Nope!"
I'm biased by the fact that this (referring to how this option in this PR makes things work) is how post permalinks worked by default in ye olde 2013—or, actually, before the post header update that this Tweaks option was made to combat.
In my eyes, this option's intention (or at least my intention behind merging it) was to pin the permalink behaviour to that of pre-Redpop. Blog names double as post permalinks, reblogged-from names double as parent post permalinks, and every name in the reblog trail doubles as its associated trail item's permalink. That last part is still how the blog network behaves if you don't use {block:Reblogs} in your theme! And because we already have post permalinks, parent post permalinks, and trail item permalinks... this makes the non-text-linked permalinks redundant—there is no other type of permalink you could want. And redundant elements/behaviour in the UI exist to be removed.
"Move post links to the blog name in the header" is technically a little iffy on grammatical tense, probably, but so is the fact that we're referring to both post and trail item headers...
In my eyes, this option's intention (or at least my intention behind merging it) was to pin the permalink behaviour to that of pre-Redpop.
Joke joke it's a joke: "oh, so nostalgia features are allowed if the repository owner likes them, I see :3"
Blog names double as post permalinks, reblogged-from names double as parent post permalinks, and every name in the reblog trail doubles as its associated trail item's permalink.
I find "doubles as" an interesting wording here. These names certainly are serving as post permalinks, but it's at the expense of serving as blog links! (Yes, the avatar is a blog link, so we still have one; that's about as arcane as the good old "the timestamp is a permalink" thing.)
Anyway, I do think making a feature a straight revert is fine. (You still do clearly get taken to the blog you clicked on, it's just a certain post on that blog. That does allow us to—and probably encourages us to—make the feature's copy explicitly about that fact, which "Classic Permalinks" certainly would. (This feature already was doing that, with "Restore," which I always thought was erroneous; a user from after the change would be confused.)
I can't really think of a way to make "restore" work here though. Move back? But "Move post links back to the blog name in the header" is... just the same thing that I just said.
@marcustyphoon Can we agree to shelve the naming discussion for now? I do plan on promoting this option to its own feature, so we can debate more thoroughly over the feature description then. Right now, I just want to be sure that there are no issues that block this from being included in Monday's release.
(Although, if you think a849c4515cee5c1acf2cefac8d24e7ace6b218b7 is a step backwards, feel free to revert it.)
Move post permalinks to blog names, Ã la 2016
I was going to put the gunshow I guess meme here, but I forgot he's frowning and I can't be bothered to try to badly photoshop a laughing version :D
Can we agree to shelve the naming discussion for now?
Sure, I think this is fine. ("Move permalinks back to" would also work.) If we weren't considering promoting it I would probably suggest we try to be sure, as I don't want to change a feature name an additional time unnecessarily.
I just want to be sure that there are no issues that block this from being included in Monday's release.
I was debating this. The reason to consider delaying it (not necessarily until release) would be that the feature is still being actively worked on on a day-to-day basis; a) delaying could dodge a https://github.com/AprilSylph/XKit-Rewritten/pull/1904#issuecomment-3382714072 situation (note: that was a post-release change, so not directly analogous), and b) delaying gives more time for the enthusiast Tumblr users who run our code to encounter the changes as they occur, give staff feedback about them to improve the feature for all Tumblr users, and just be aware of them in order to better inform other users. (For example: double-click-and-drag text selection works now sometimes? But it seems kind of inconsistent. I'm trying to figure out an accurate reproduction method.)
As we've discussed before, the obvious counterargument is that users who go out of their way to enable a revert feature are probably for the most part pretty set in their opinion that the new UI is simply bad/unredeemable/etc. This mostly applies here, I think, though this PR is notably—for the people, like me, who had the tweak in question enabled—an unconditional opt-out of the A/B, and for those who don't follow the addons blog, it will be effectively irreversible, since there's no reason to think that it's XKit doing it rather than the A/B simply ending on your account, and thus no reason to check.
(edit: Well, and the fact that clickable posts are simply annoying and feedback is probably not that likely to ultimately meaningfully change them is certainly another obvious counterargument.)
I was going to put the gunshow I guess meme here, but I forgot he's frowning and I can't be bothered to try to badly photoshop a laughing version :D
*peeks out from behind a pole* Admittedly, that wasn't what I meant, buuuut :P
Edit: No, wait: "peeks out from over a shelf," would have been the correct joke.
Well, now you have three options to choose from. You have until Monday morning.
The reason to consider delaying it (not necessarily until release) would be that the feature is still being actively worked on on a day-to-day basis
The AMO review time situation means we're essentially trying to predict what the situation will be in a week's time no matter what, so trying to avoid the need to hotfix an unreleased version or do damage control on a version that breaks the same day it's released... is a game I don't feel is worth playing. I would rather execute a plan for the best-case scenario and have that pay off, than to eschew that in the spirit of utmost caution—at least in my current situation, where I have essentially unlimited time to give to the project.
though this PR is notably—for the people, like me, who had the tweak in question enabled—an unconditional opt-out of the A/B, and for those who don't follow the addons blog, it will be effectively irreversible, since there's no reason to think that it's XKit doing it rather than the A/B simply ending on your account, and thus no reason to check.
I acknowledge that this point is valid, but I feel strongly enough about what my intentions for adding this tweak are/were and feel safe enough in my assumptions about the intentions of the majority of people who enabled it are/were that my position of "let's ship this in the next release" remains unchanged.
Well, now you have three options to choose from.
No, no, it's on the shelf now! I'm very short, I cannot reach up there.
I would rather execute a plan for the best-case scenario and have that pay off...
General agreement, which I will caveat with the fact that I do not consider this the best case scenario. Barring Staff coming up with something better than I've thought of so far, I consider feedback causing this part of the the A/B to get killed and us thus not needing to ship this PR at all the best case scenario.
(Anyway, I haven't prioritized going through the full test list in order to fully approve this yet, as I figure it makes sense to press that button after the end of the workweek when Staff presumably stops pushing releases. Should have it done tomorrow barring finding a bug.)
- with a/b test flag:
- [x] post header/body does nothing
- post header blog soft navigates...
- [x] in general
- [ ] to @ blog
- post header blog command click hard navigates...
- [x] to peepr
- [x] to custom theme
- [x] to custom theme with custom domain
- attribution blog soft navigates...
- [x] in general
- [ ] to @ blog
- attribution blog command click hard navigates...
- [ ] to peepr: kind of
- [x] to custom theme
- [x] to custom theme with custom domain
- [x] trail header/body does nothing
- trail header blog soft navigates...
- [x] in general
- [x] to @ blog
- trail header blog command click hard navigates...
- [x] to peepr
- [x] to custom theme
- [x] to custom theme with custom domain
- [x] contributed content header/body does nothing
- contributed content header blog soft navigates...
- [x] in general
- [ ] to @ blog
- contributed content header blog command click hard navigates...
- [x] to peepr
- [x] to custom theme
- [x] to custom theme with custom domain
- [x] to community post
- without a/b test flag:
- [x] post header does nothing
- [x] trail header/body does nothing
- [x] contributed content header/body does nothing
Interestingly, rebloggedFromUrl uses blogname.tumblr.com even when the blog in question doesn't have a custom theme, so we don't avoid a redirect when command clicking reblog attribution that leads to a non-custom theme blog; I don't see a way we could, and it works totally fine.
Side note that this doesn't work in radar (like classic footer now does); I don't know that it'd be worth bothering to do. But I guess an argument could be made that it makes sense for consistency.