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

feat(contacts-menu): implement custom javascript hook action #49375

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

st3iny
Copy link
Member

@st3iny st3iny commented Nov 19, 2024

Summary

Adds a new frontend-only contacts menu action registered via registerContactsMenuAction("foo-bar") from @nextcloud/vue.

Note: Please focus on the contact menu changes when reviewing this PR. The modal you see might change and is part of the Calendar app.

2024-11-27.14-35-29.mp4

Checklist

Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to register this in the backend?

One the one side we already have link actions, but on the other link actions make sense for php-only apps and javascript actions would always need JS code.

Asking because all other APIs we introduced lately for similar things are purely frontend. E.g. file actions, views, filters.
Also older API is frontend only (e.g. user list rows, user actions etc).

@st3iny st3iny force-pushed the feat/contacts-menu/js-hook-action branch from a40d301 to a579d7c Compare November 25, 2024 16:56
@st3iny
Copy link
Member Author

st3iny commented Nov 25, 2024

Does it make sense to register this in the backend?

One the one side we already have link actions, but on the other link actions make sense for php-only apps and javascript actions would always need JS code.

Asking because all other APIs we introduced lately for similar things are purely frontend. E.g. file actions, views, filters. Also older API is frontend only (e.g. user list rows, user actions etc).

Hmm, that is actually a good point. But since I'm extending an existing API here, I'd prefer to keep it in the backend for now. Otherwise, it might get confusing as the avatar menu is populated from multiple sources of truth.

I'll think about it again.

@st3iny st3iny force-pushed the feat/contacts-menu/js-hook-action branch from a579d7c to b48320c Compare November 26, 2024 20:42
@st3iny
Copy link
Member Author

st3iny commented Nov 26, 2024

Ferdinand is right. The OCP API doesn't make sense. The menu has to registered from the frontend only, via @nextcloud/vue.

:key="idx"
:key="`${idx}-link`"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated, but my LSP complained that it is illegal to have the same key on the on both nodes of v-if/v-else.

@st3iny st3iny marked this pull request as ready for review November 26, 2024 20:45
@st3iny st3iny added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants