nostr
/
nostrweb
3
1
Fork 1
Code Issues 24 Pull Requests 3 Projects 1 Releases 27 Activity

Labels Milestones

feed: display link preview with noxy #34

Merged
offbyn merged 4 commits from feed-preview-links into master 2 years ago
Conversation 8 Commits 4 Files Changed 4
offbyn commented 2 years ago
Owner

fetches the first link and shows it below a text note, all
resources are prxied through noxy.

see nostr/noxy#2

fixes #30

fetches the first link and shows it below a text note, all resources are prxied through noxy. see https://git.qcode.ch/nostr/noxy/pulls/2 fixes https://git.qcode.ch/nostr/nostrweb/issues/30
offbyn added 1 commit 2 years ago
ci/woodpecker/push/woodpecker Pipeline was successful Details
ci/woodpecker/pr/woodpecker Pipeline was successful Details
f411298129
feed: display link preview with noxy 
fetches the first link and shows it below a text note, all
resources are prxied through noxy.

see nostr/noxy#2
offbyn requested review from x1ddos 2 years ago
x1ddos reviewed 2 years ago
x1ddos left a comment
Owner

would it be possible to load only those images which are in the current view port?

one issue is, on a frech page load, nostrweb loads lots of events and hits noxy with many requests, resulting in "429 too many requests" rate limit and broken images.

would it be possible to load only those images which are in the current view port? one issue is, on a frech page load, nostrweb loads lots of events and hits noxy with many requests, resulting in "429 too many requests" rate limit and broken images.
src/main.js Outdated
@ -160,0 +171,4 @@
function linkPreview(href, id, relay) {
if ((/\.(gif|jpe?g|png)$/i).test(href)) {
return elem('div', {},
[elem('img', {className: 'preview-image-only', src: getNoxyUrl('data', href, id, relay).href})]
x1ddos commented 2 years ago
Owner

when noxy is hit with too many requests, it'll respond accordingly, with 429 status code. would it make sense to attache a "on error" handler and retry later? something like this in a pseudo-code:

img = elem('img', ...);
img.addEventListener('error', handleImageLoadFailure);

function handleImageLoadFailure(event) {
  // event.target is img?
  const img = event.target;
  const retryCount = img.data('retry-count');
  if (retryCount > 3) {
    // remove 'error' handler from the img element
    // remove img preview from the dom
    return;
  }
  setTimeout(() => {
    img.src = img.src?ts=time.now(); // force reload with a fresh timestamp
  }, retryCount * 1000 /* backoff interval */);
}
when noxy is hit with too many requests, it'll respond accordingly, with 429 status code. would it make sense to attache a "on error" handler and retry later? something like this in a pseudo-code: ```js img = elem('img', ...); img.addEventListener('error', handleImageLoadFailure); function handleImageLoadFailure(event) { // event.target is img? const img = event.target; const retryCount = img.data('retry-count'); if (retryCount > 3) { // remove 'error' handler from the img element // remove img preview from the dom return; } setTimeout(() => { img.src = img.src?ts=time.now(); // force reload with a fresh timestamp }, retryCount * 1000 /* backoff interval */); } ```
👍 1
x1ddos commented 2 years ago
Owner

and a similar approach for preview below.

and a similar approach for preview below.
offbyn commented 2 years ago
Poster
Owner

I'll try, maybe there is a way without appending ?ts=time.now(), asuming you don't send cache headers on 4xx responses.

I'll try, maybe there is a way without appending `?ts=time.now()`, asuming you don't send cache headers on 4xx responses.
x1ddos commented 2 years ago
Owner

right, noxy doesn't send cache-control in 4xx but will a browser actually make a new request without a ts?

right, noxy doesn't send cache-control in 4xx but will a browser actually make a new request without a `ts`?
x1ddos commented 2 years ago
Owner

amazing when it works! this looks very nice:

image

amazing when it works! this looks very nice: ![image](/attachments/5df4e13b-6145-49e7-8647-dcbff2f77f63)
image.png
121 KiB
x1ddos commented 2 years ago
Owner

would it be possible to load only those images which are in the current view port?

one issue is, on a frech page load, nostrweb loads lots of events and hits noxy with many requests, resulting in "429 too many requests" rate limit and broken images.

here you see on my feed, noxy started responding with "429 too many requests" at some point:

image

image

> would it be possible to load only those images which are in the current view port? > > one issue is, on a frech page load, nostrweb loads lots of events and hits noxy with many requests, resulting in "429 too many requests" rate limit and broken images. here you see on my feed, noxy started responding with "429 too many requests" at some point: ![image](/attachments/4130303b-d7d1-4d07-98a8-ffdf53117658) ![image](/attachments/8e50f363-c28d-4f7d-98cf-2dbacd81764c)
image.png
103 KiB
image.png
95 KiB
offbyn commented 2 years ago
Poster
Owner

would it be possible to load only those images which are in the current view port?

💯 definetly, to keep it super simple in this iteration I'll try to just add loading="lazy" attribute to the images, https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img#attr-loading

> would it be possible to load only those images which are in the current view port? 💯 definetly, to keep it super simple in this iteration I'll try to just add loading="lazy" attribute to the images, https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img#attr-loading
offbyn commented 2 years ago
Poster
Owner

attache a "on error" handler and retry later?

I want, but need to play around with this about how to do it exactly, currently the request is created by <img src="noxy-url", so I am not sure what to do onerror, maybe just removing and adding the image from the DOM will try to request it again. alternativelly we could fetch in js and on success add the image to the DOM, then the question is should it even try to ever load by img src or always from js.

> attache a "on error" handler and retry later? I want, but need to play around with this about how to do it exactly, currently the request is created by `<img src="noxy-url"`, so I am not sure what to do onerror, maybe just removing and adding the image from the DOM will try to request it again. alternativelly we could fetch in js and on success add the image to the DOM, then the question is should it even try to ever load by img src or always from js.
x1ddos approved these changes 2 years ago
x1ddos left a comment
Owner

lgtm! let's merge now and improve later, but before v1.0.0 :)

lgtm! let's merge now and improve later, but before v1.0.0 :)
offbyn force-pushed feed-preview-links from f411298129 to 80944adef7 2 years ago
offbyn commented 2 years ago
Poster
Owner

rebased

rebased
offbyn added 3 commits 2 years ago
a1c2ca4d41
noxy: add lazy load attribute to noxy images 
Only for browsers that support loading=lazy attribute, but this
should reduce the load on noxy and only fetch images that are
relevant (in the viewport of close to it).
ci/woodpecker/push/woodpecker Pipeline was successful Details
ci/woodpecker/pr/woodpecker Pipeline was successful Details
52b16b75d2
preview: add primitive fetch que and wait for noxy response 
Instead of just blindly fetch noxy requests, this adds a simple
fetch que that waits until the pending response is done and
continues with the next one whichever seen first, this is not
in chronological order, but incoming event order (incoming events
from different relays).
offbyn force-pushed feed-preview-links from 52b16b75d2 to 1a71ae2707 2 years ago
offbyn force-pushed feed-preview-links from 1a71ae2707 to cd71da0a87 2 years ago
offbyn commented 2 years ago
Poster
Owner

@x1ddos I added lazy loading for images and a queue to fetch metadata (this is very rundimentary and could be improved in various ways in the future).

@x1ddos I added lazy loading for images and a queue to fetch metadata (this is very rundimentary and could be improved in various ways in the future).
🎉 1
offbyn force-pushed feed-preview-links from cd71da0a87 to b321acde02 2 years ago
offbyn commented 2 years ago
Poster
Owner

rebased

rebased
x1ddos commented 2 years ago
Owner

nice! tried locally, works wonderfully. ship it!

nice! tried locally, works wonderfully. ship it!
offbyn self-assigned this 2 years ago
offbyn merged commit b321acde02 into master 2 years ago
offbyn deleted branch feed-preview-links 2 years ago

Reviewers

x1ddos approved these changes 2 years ago
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
ci/woodpecker/push/woodpecker Pipeline was successful
Details
The pull request has been merged as b321acde02.
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b feed-preview-links master
git pull origin feed-preview-links

Step 2:

Merge the changes and update on Gitea.
git checkout master
git merge --no-ff feed-preview-links
git push origin master
Sign in to join this conversation.
Reviewers
Request review
No reviewers
x1ddos
Labels
Apply labels
Clear labels
  
bug
   
duplicate
   
feature
   
halp!
   
invalid
   
wontfix
No Label
bug
duplicate
feature
halp!
invalid
wontfix
Milestone
Set milestone
Clear milestone
No items
No Milestone
Projects
Set Project
Clear projects
No project
Assignees
Assign users
Clear assignees
No Assignees
offbyn
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: nostr/nostrweb#34
Write Preview
Loading…
Cancel
Save
There is no content yet.
Delete Branch 'feed-preview-links'

Deleting a branch is permanent. It CANNOT be undone. Continue?

No
Yes