-
Notifications
You must be signed in to change notification settings - Fork 49
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
base: master
Are you sure you want to change the base?
Mutual Checker, NotificationBlock, Quote Replies: Process new reply format #1659
Conversation
961d8e3
to
c42de4b
Compare
5ae2b74
to
49ff16e
Compare
49ff16e
to
62ee14e
Compare
transform: scale(0); | ||
} | ||
|
||
${notificationSelector}:is(:hover, :focus-within) button.xkit-quote-replies svg { |
There was a problem hiding this comment.
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.
Re: Quote Replies, see also: #1662. |
brain dump of my current thinking about how to fetch specific replies:
also |
Wait, there is a timestamp... hm, why did I think there wasn't a timestamp? I have screenshots of the API data from when I was working on this and there absolutely is a timestamp that I must have missed. (Unless that's the post timestamp or something absurd, but why would that be the case). Er, stay tuned. |
Co-Authored-By: April Sylph <[email protected]>
2c03b8b
to
bc1da02
Compare
Done. (I assume there's no way for a
Done. Well, sort of; I call the existing code here, which assumes that the first reply with a matching timestamp is the right one. We should... probably add that partial match check?
As implemented, this falls back on any failure of the processing code. |
|
||
const targetPostSummary = bodyDescriptionContent.text.slice(summaryFormatting.start + 1, summaryFormatting.end - 1); | ||
|
||
return await processReply({ type, timestamp, targetPostId, targetTumblelogName, targetPostSummary }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be two lines (const { content, tags } = await processReply(/* */); return { content, tags };
)? I know some people instinctively recoil at "return await" but it's necessary here to trigger the synchronous catch block.
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.
outdated Quote Replies info
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
To easily distinguish affected activity items, run
document.documentElement.append(Object.assign(document.createElement('style'), { textContent: '[aria-label="Notification"]:not([role="listitem"]) { outline: 1px solid red; outline-offset: -3px }' }))
.Mutual Checker:
NotificationBlock:
Quote Replies: