Add last-notification-page-button feature
#6133
Conversation
yakov116
changed the title
linkify-notification-page-numbers featurelast-notification-page-button feature
| const lastCursor = Math.floor(lastNotificationPageNumber / 50) * 50; | ||
| const nextButtonSearch = new URLSearchParams(nextButton.search); | ||
| nextButtonSearch.set('after', btoa('cursor:' + String(lastCursor))); | ||
| nextButton.search = nextButtonSearch.toString(); |
There was a problem hiding this comment.
This changes the next button too. Please check my suggestions carefully, I wrote some specific code that isn't this. Let's not do 40-comments reviews again :(
| nextButton.search = nextButtonSearch.toString(); |
There was a problem hiding this comment.
I fail to see how that would not work. String(nextButtonSearch) is exactly the same as nextButtonSearch.toString() and the search is exactly what we need.
This line is definitely wrong in any case.
There was a problem hiding this comment.
Now I understand what you are saying. 👍 Never mind this did not work
There was a problem hiding this comment.
Your original suggestion was to remove the nextButtonURL, which I did.
If I don't include this line then the URL will only be the search, https://github.com/after=Y3Vyc29yOjEwNTA%3D&query=is%3Adone missing the notifications URL.
I feel bad we are going though so many comments.
If you feel that I am missing something, I have no issue with you closing and allowing someone else to pick this up. (No hard feelings I promise)
There was a problem hiding this comment.
Gotcha, the only misunderstanding here is that ? is missing when using String() whereas it exists in url.search. We just need to prepend the question mark so that the attribute will be exactly "?after=Y3Vyc29yOjEwNTA%3D&query=is%3Adone", which is relative to the current pathname.
Possible ways, depending on which one TS likes the most
href={'?' + String(nextButtonSearch)} href={`?${nextButtonSearch}`}
Co-authored-by: Federico Brigante <me@fregante.com>
Co-authored-by: Federico Brigante <me@fregante.com>
There was a problem hiding this comment.
I just remembered this is a dynamic page so I think you'll need to use observe rather than a regular select I think. The listener function should be at the top level (outside init)
To trigger this, mark 10+ notifications as "Done" and wait for the page to update.
| } | ||
|
|
||
| const lastNotificationPageNode = select('.js-notifications-list-paginator-counts')!.lastChild!; | ||
| assertNodeContent(lastNotificationPageNode, new RegExp(/^of \d+$/)); |
| import {assertNodeContent} from '../helpers/dom-utils'; | ||
|
|
||
| function init(): void | false { | ||
| const nextButton = select('a[data-hotkey="ArrowRight"]'); |
There was a problem hiding this comment.
This attribute is added by us
Closes #6126
Test URLs
https://github.com/notifications?query=is%3Adone
Screenshot