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

Profile Gallery #973

Merged

Conversation

believethehype
Copy link
Contributor

@believethehype believethehype commented Jul 5, 2024

Here is an alternative attempt at profile galleries. This is an alternative to #957 and only one of both PRs should be used.

The main difference:

This uses kind 1163 (for now) Events per Gallery Entry (Similar to 1063 but with e tag to an event with fileheader info only optional) instead of kind 10011 lists. Here the events represent gallery entries and an infinite amount can exist, where for lists there might be limited by size and have a potential chance to get wiped.

The current state of the PR works in general (add, delete, show) but it seems it's sometimes having issues to load single events (especially older ones). Not sure if thats a limitation of the filter or if there's some easy way to fix this (Didn't seem to happen for lists)

One remaining current limitation is that when a note gets deleted and a gallery entry references to that note, Amethyst will not know and show a loading icon with currently no possiblity to remove the entry

…shows profile tab

What works:
- can load existing 10011 lists and show images in profile tab

What doesn't work:
- gallery view is broken
- can't load notes to click images to get to original note
- adding media to gallery crashes amethyst atm
- no functionality to delete media from gallery yet.
@believethehype believethehype changed the title Profile Gallery alternative version Profile Gallery Jul 7, 2024
@vitorpamplona
Copy link
Owner

So, is this one ready to go? Or are we waiting for the NIP repo to move merge?

@believethehype
Copy link
Contributor Author

I had withdrawn my previous PR on the NIPs repo as this was the approach with lists (NIP51) .
I think this one is the better approach.

I haven't made a new PR since. Basically what we need is to reserve the 1163 (or whatever we decide) Kind with an url tag and an e tag.

I don't know if this should be part of an existing NIP or a new NIP (it's a pretty simple addition, yet it should be written somewhere sooner or later).

In terms of Code, I believe this is ready to go, except you have any suggestions :)

@believethehype believethehype marked this pull request as ready for review July 10, 2024 17:35
@vitorpamplona vitorpamplona merged commit 17109e7 into vitorpamplona:main Jul 10, 2024
1 check passed
@vitorpamplona
Copy link
Owner

Nice!

Basically what we need is to reserve the 1163 (or whatever we decide) Kind with an url tag and an e tag.

That would be a small nip just to define the kind and tags

@believethehype
Copy link
Contributor Author

I made a new PR on the nips repo.

nostr-protocol/nips#1356

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.

2 participants