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

External event handlers #895

Open
dtonon opened this issue Oct 18, 2024 · 19 comments
Open

External event handlers #895

dtonon opened this issue Oct 18, 2024 · 19 comments
Labels
UI/UX Related to interface and user experience

Comments

@dtonon
Copy link
Collaborator

dtonon commented Oct 18, 2024

A first proposal for the external event handlers.
Compared to the current base version there are some technical/functional additions:

  • The number of applications in the main list, with a hint of the activated ones
  • The list of users who recommended the app
  • A single button to recommend/revert

image

image

/cc @mikedilger @bu5hm4nn

@dtonon dtonon added the UI/UX Related to interface and user experience label Oct 18, 2024
@dtonon
Copy link
Collaborator Author

dtonon commented Oct 18, 2024

Import feature:

image

@mikedilger
Copy link
Owner

This is all good except for the "who recommended." I don't think it is important. And it makes each list entry taller meaning fewer fit on the screen at once. If people really are curious who recommended, we could do a query over the events to find it on the press of a button or something. Otherwise we are creating another database table (EventKind, HandlerKey) -> PublicKey (requiring duplicate keys) to get fast enough to render your example.

I like that we are separating the "Name" from their metadata, but also giving the URL hostname regardless. This will help people detect imposter apps.

I'm OK with recommend-by-button. Each press will regenerate the event for that 'kind'. This is how I started, but then I switched to the per-page recommend button so that you could change multiple things without posting an event yet, and then just make the event when you are ready. But that leaves things out-of-sync and is such a micro-optimization that I think your way is better.

BTW: All recommendations by default are enabled currently. Maybe we can rethink that later.

The event kind names DontHaveSpaces currently, but I could make a change to fix that.

@mikedilger
Copy link
Owner

Ok I added spaces to the event kinds by updating nostr-types.

I also redefined the name functions to be:

  • metaname() -> Option<String> - the name from the embedded Metadata
  • hostname(kind) -> Option<String> - the hostname of the URL for that event kind
  • bestname(kind) -> Option<String> - the metadata name, or if missing, the hostname for that event kind

Since handlers have up to 2 different URLs (we only keep 'nevent' and 'naddr') we need to know which one you wanted the hostname from, and so passing in the kind (which is always known in context) lets us choose.

@bu5hm4nn
Copy link
Collaborator

bu5hm4nn commented Oct 18, 2024

@dtonon Handlers also have metadata just like profiles:

pub struct MetadataV1 {
    /// username
    pub name: Option<String>,

    /// about
    pub about: Option<String>,

    /// picture URL
    pub picture: Option<String>,

    /// nip05 dns id
    pub nip05: Option<String>,

    /// Additional fields not specified in NIP-01 or NIP-05
    pub other: Map<String, Value>,
}

@bu5hm4nn
Copy link
Collaborator

@mikedilger Currently I just count all the handlers for the numbers in daniele's design, but then when you click in the way you had programmed the loop we reject those that don't have metadata (for example kind 0 has 11 handlers but when I click in I can only configure 4 of them). How do we want to reconcile that?

@mikedilger
Copy link
Owner

mikedilger commented Oct 19, 2024

How do we want to reconcile that?

I think I will make the process.rs code reject them early, so we don't save handlers that don't pass the validation. Then I will make a schema update that erases all broken handlers. Then the count will be in sync.

EDIT: I have done this. The counts are fixed now.

@dtonon
Copy link
Collaborator Author

dtonon commented Oct 19, 2024

@mikedilger

we could do a query over the events to find it on the press of a button or something.

That's fine with me.
I'm updating the design.

I'm OK with recommend-by-button. Each press will regenerate the event for that 'kind'.

I also thought about the mass update, but thinking about a real pattern use I suppose that the user will toggle single elements every now and then. So this interface is simpler and immediate.

All recommendations by default are enabled currently. Maybe we can rethink that later.

There is no harm in this, and helps to surface and promote new Nostr apps.

@dtonon
Copy link
Collaborator Author

dtonon commented Oct 19, 2024

@bu5hm4nn

Handlers also have metadata just like profiles:

Are they actually used?

@dtonon
Copy link
Collaborator Author

dtonon commented Oct 19, 2024

we could do a query over the events to find it on the press of a button or something.

I would follow the Relays list interface:

  • When the mouse over the row the color is slightly changed;
  • On click the row expands and show more content; here I added an about, from the metadata;
  • The close button is not necessar, when another row is clicked the previous is closed; we can close it also clicking on the same row;

image

@dtonon
Copy link
Collaborator Author

dtonon commented Oct 19, 2024

This is how I would handle the case of the unknown event:

Screenshot 2024-10-19 at 10 42 21

image

@mikedilger
Copy link
Owner

@bu5hm4nn

Handlers also have metadata just like profiles:

Are they actually used?

Yes. Many of them have it.

@dtonon
Copy link
Collaborator Author

dtonon commented Oct 19, 2024

Yes. Many of them have it.

As you can see from the mockup I added the "about" field, we can integrate more details if needed.

@mikedilger
Copy link
Owner

Ok I added code to show who recommended the handler. I think it runs fast enough to just show it everywhere.

@dtonon
Copy link
Collaborator Author

dtonon commented Oct 19, 2024

Ok I added code to show who recommended the handler. I think it runs fast enough to just show it everywhere.

@bu5hm4nn so we can skip the row opening, just follow the design of the expanded row.

@bu5hm4nn
Copy link
Collaborator

Many of the changes are implemented now.

One thing we still need to reconcile is that "recommend" is currently a bool switch per handler and there is one central "Share recommendations" button. Is the backend logic going to change to individual recommendations or should we just fake this behavior from the Ui so that it behaves the way @dtonon has designed it: We would just change the bool and publish the new recommendations (all of them).

@mikedilger
Copy link
Owner

You always have to publish all your recommendations per-kind. So if there are lots of buttons instead of switches, each one publishes that entire kind (all the rows?). The backend does not change, it cannot publish them separately.

By making them buttons, the UI state never gets out of sync with what is published. By using switches and local state, it gets out of sync and then they have to press the button at the top. So I conceded to Daniele's many-button design. It doesn't hurt to publish every time they press a button.

@dtonon
Copy link
Collaborator Author

dtonon commented Oct 23, 2024

@bu5hm4nn in addition to Mike's indication on the buttons, some minor tweaks:

  • If all apps are active just show the number ("X apps" instead of "X of X apps")
  • if no apps are selected show "zero of X apps", but in gray
  • Change the row's color on mouse over (added 2024-10-24)
  • Move the recommended before the about
  • Remove "About:" in front of the text
  • Add spacing between the handler details

You can find all these details alsi in the design (figma).

@bu5hm4nn
Copy link
Collaborator

@dtonon Which count does "X of X apps" refer to? How many are enabled or how many we recommend?

@dtonon
Copy link
Collaborator Author

dtonon commented Oct 24, 2024

@bu5hm4nn "(enabled) of (recommended) apps", as it is now is correct, it's just an update in the design.
Were you able to check the note rendering as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI/UX Related to interface and user experience
Projects
None yet
Development

No branches or pull requests

3 participants