Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mutual Checker, NotificationBlock, Quote Replies: Process new reply format #1659

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

marcustyphoon
Copy link
Collaborator

@marcustyphoon marcustyphoon commented Dec 5, 2024

Well, #1658 is fairly problematic.

Description

This contains (somewhat rudimentary) fixes for the linked issue, in which the relevant features are broken by certain kinds of activity item that use a new internal data format, containing only enough information to render the notification rather than all of the internal data we use to add functionality.

  • Mutual Checker can still easily determine if the replying user is a mutual, since Tumblr renders that.
  • NotificationBlock wants to know the root post of the target post, which just so happens to work in this case because the activity items in question are on original posts, but will be a problem in the future if all activity items switch to this API.
  • Quote Replies... is a somewhat complicated scenario because we might technically have enough information to look up the relevant post and post activity using an API fetch, but only via very convoluted methods (the activity item links to the post; we could parse the url to get the relevant information to API fetch the post but we have no timestamp with which to easily find the reply). For now, just to make it work on all posts rather than keep getting confused users, I've processed the activity item itself into a rudimentary post body:

This truncates the reply text and generally looks rather stupid, but it's better than nothing. Note also that it does not currently exclude community posts (I didn't see a way to do so). I did test the code on a reply on a community post.

Testing steps

not yet documented

@marcustyphoon marcustyphoon force-pushed the new-notification-fix branch 3 times, most recently from 5ae2b74 to 49ff16e Compare December 9, 2024 23:47
@marcustyphoon marcustyphoon changed the title partial new notification fix Mutual Checker, NotificationBlock, Quote Replies: Process new reply format Dec 9, 2024
@marcustyphoon marcustyphoon marked this pull request as ready for review December 10, 2024 00:02
transform: scale(0);
}

${notificationSelector}:is(:hover, :focus-within) button.xkit-quote-replies svg {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably should have done the switch to styleElement in two commits, now that I think about it. For reference, this should be the only changed line in the CSS.

@marcustyphoon
Copy link
Collaborator Author

Hm... maybe this partial implementation of Quote Replies should include an informational modal each time it's used?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant