XKit-Rewritten icon indicating copy to clipboard operation
XKit-Rewritten copied to clipboard

New Feature: Mute

Open marcustyphoon opened this issue 3 years ago • 10 comments

User-facing changes

  • As per the specification in the linked issue, adds a script that selectively mutes original posts, reblogged posts, or all posts from a user across Tumblr, with controls accessible from the meatball menu as well as within XKit's settings menu.

This departs slightly from the linked specification in a few ways:

  • An interactive informational/warning element with the ability to temporarily show all posts is placed at the start of the timeline in the blog view modal if the user in question is muted in any mode, not just in "all posts" mode. (Along with being a nice feature, the special case logic got really weird looking otherwise). The ability to show posts anyway in the modal is applied only to posts muted because of that user (onBlogHiddenClass).
  • Muted blogs, despite being key-value, are stored as an array of [key, value] entries instead of in an object. Web extension storage does not preserve insertion order on objects, annoyingly.
  • The UUID -> blog name dictionary is stored as a separate object, to guarantee that the race condition mentioned in the specification does not occur no matter when the script updates the dictionary.
  • Minor UI stuff ("unmute" is a button in the preference iframe, etc).

~~Does not currently implement a meatball menu button on blogs themselves; see #910.~~

Technical explanation

See comment history.

Also, note that posts are hidden via visibility: hidden instead of display: none in "hide everything on this blog" mode so that they take up space and Redpop doesn't just try to load posts forever until the tab runs out of memory.

Issues this closes

resolves #674

marcustyphoon avatar Jul 28 '22 07:07 marcustyphoon

todo:

  • ~~implement handling for changed blog names~~
  • ~~rethink warning element insertion and state; current method is probably not viable even with refactoring~~
  • ~~"when hiding all posts from that blog, no posts should be hidden" - oops didn't see the word "all" there~~
  • ~~handle the single post in the blog view being muted (maybe this is fine as-is?)~~
  • ~~handle nothing being muted in the options ui~~
  • literally any ui css / script description, though that wasn't part of the spec per se

The most nontrivial things, I think are:

  • ~~Keeping the UI state in sync in the blog view modal is just a huge pain (window.mutedBlogsUserHasClickedShowOn, anyone?)~~
  • It's hard to translate between name and uuid in the blog view modal (I thought of calling timelineObject on a post in processTimelines, ~~but that sounds silly~~ and tried it)
  • Tumblr differentiates between the main blog and the root blog, but we care about the original blog and the reblogging blog, which maps to those differently depending upon whether the post is original or not; this requires some weird looking conditionals

marcustyphoon avatar Jul 28 '22 07:07 marcustyphoon

The blog name should be updated whenever the mute modal is opened for a muted blog - not when processing posts, despite the opportunity existing. (It is potentially problematic to have both a passive and an active way to affect the same storage key, as one may overwrite the other.)

One potential fix for this, I think, is to have two separate storage keys: one for uuid => is this muted or not, and one for uuid => what blog name corresponds to this uuid. Thus, passive updates to the name dictionary cannot overwrite a simultaneous attempt to mute/unmute a blog.

The format for the feature's storage I have in mind is to have one object stored on mute.mutedBlogs. Blog UUIDs will be the object's keys, each with an array value of the blog's last seen name and applied mute setting, e.g. "t:Fft_7iTGnBlHdl-UtlllwQ": ["april", "reblogs"].

One drawback of this is that the order blogs you have blocked will appear arbitrary; it seems like local storage does not preserve object key order, and so the entries appear sorted alphabetically by UUID, which is sort of weird.


edit: in 6cae4f2, I tried: a) Using two storage keys, each a single dictionary with uuid keys. not sure how to bug test this, but this should allow instantaneous updates of changed blog names without the potential for overwriting mute mode data b) Actually storing the mute mode data as an array using Object.entries() on set and Object.fromEntries() on get. This ensures insertion order is preserved, which feels a lot less weird than sorting alphabetically by uuid or blog name. Kind of weird, though?

marcustyphoon avatar Aug 04 '22 04:08 marcustyphoon

As of 26560e9, this currently uses a somewhat weird setup in which processPosts writes the blog UUIDs into post data attributes, and the logic for whether posts are hidden is done in CSS rules.

This means that posts do not have to be reprocessed if the user's muted blogs change; the CSS rule is changed instead. This doesn't seem like a significant benefit to me; I actually tried this to try to solve a bunch of state issues that I've now realized this method isn't necessary to solve. I don't know if there's really a downside either, though, besides "that's pretty odd."

I guess less CSS rules at the expense of a potentially very slightly longer page stutter when you mute someone with a ton of posts on screen is probably better, so I'll probably go back to the "normal" way of having a hiddenclass, I guess?

(That commit also uses the state logic from Show Originals of adding a DOM element that controls the mode a timeline is in and comparing the timeline to the properties of the DOM element to figure out if navigation has occurred, but I have no interesting comment on that).


~~edit: meh, I dunno, actually; it's pretty hard to read the other way~~

edit: removed in 7e106e3

marcustyphoon avatar Aug 04 '22 06:08 marcustyphoon

I was going to write a todo item here for removing the code that handles timeline elements whose timeline properties are UUIDs, but since I really do need to know the UUID of a timeline in the blog view, the weirdly complicated code has to stay for now and this removes 0 lines.

marcustyphoon avatar Sep 02 '22 17:09 marcustyphoon

On the blog view, when hiding all posts from that blog, no posts should be hidden - instead, the entire timeline should be hidden, and an interactive warning put in its place, informing the user that this blog is muted, with a button to show its posts anyway.

Maybe better and arguably simpler-to-understand form of this might be:

On the blog view of a muted blog, an interactive warning is shown above the timeline informing the user that this blog is muted (and in what mode), with a button to show its posts anyway that removes the warning.

In processing, posts are given a special class if muted because they match the current timeline and a generic one if not. Posts with the generic one are always display: none.

Special-class posts are hidden when they are preceded by the warning element. If the mode for the blog in question is "all," special-class posts are hidden with visibility to avoid infinite loading, otherwise they're hidden with display: none.

(This really doesn't simplify things all that much, honestly. What we really need is "set this CSS ruleset on elements that match ruleset a unless they match ruleset b, where both a and b can include descendants", but, y'know, that's not a thing afaik.)

marcustyphoon avatar Dec 08 '22 22:12 marcustyphoon

we should add mute-with-expire-after-n-days to the xkit rewritten mute script that doesn't exist

reminder to self to feature request issue this and assign myself if this gets merged

also

  • [x] use #908 to make labels not confusing af
  • [x] use #910

marcustyphoon avatar Feb 14 '23 18:02 marcustyphoon