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

New "getOccupantActionButtons" hook, so that plugins can add actions on MUC occupants. #3475

Merged
merged 4 commits into from
Aug 29, 2024

Conversation

JohnXLivingston
Copy link
Contributor

@JohnXLivingston JohnXLivingston commented Aug 6, 2024

Here is a new hook, so that plugins can add an action menu on occupant list.

For example, i have plugins adding following entries:

image

By default, there is no action, so no dropdown.

@jcbrand , i know that you prefer have actions in the occupant modal. But in my case, I prefer having them here.
In the future, we could use the same hook for actions in the occupant modal.

And if an action should only be shown in the modal (or only in the occupant list), we could add some attributes to the buttons, so that we can filter them.

Let me know if this PR is ok for you. (no hurry, it can wait if you don't have the time for now).

@jcbrand
Copy link
Member

jcbrand commented Aug 7, 2024

@JohnXLivingston I think the dropdown menu buttons should all be on the far right (to the right of the affiliation label), so that they're vertically aligned under one another. Right now they're shifting around horizontally which doesn't look good and makes them slightly more difficult to locate and click.

@JohnXLivingston
Copy link
Contributor Author

@JohnXLivingston I think the dropdown menu buttons should all be on the far right (to the right of the affiliation label), so that they're vertically aligned under one another. Right now they're shifting around horizontally which doesn't look good and makes them slightly more difficult to locate and click.

Yes, i tried that, but was not convinced. I will give it another try.

@JohnXLivingston
Copy link
Contributor Author

I'm not happy with the result for now. I'm not sure how to position the badges.
And if there is no action for all users, it could also have some weird behavior.

I do not have time to finish it today. I will come back to it at the end of the week (or maybe next week).
If you have any good idea until then, be my guest.

@jcbrand
Copy link
Member

jcbrand commented Aug 7, 2024

I'm not sure how to position the badges.

With display: flex and justify-content: space-between.

https://developer.mozilla.org/en-US/docs/Web/CSS/justify-content
https://getbootstrap.com/docs/5.0/utilities/flex/

@JohnXLivingston
Copy link
Contributor Author

With display: flex and justify-content: space-between.

It does not look well when the sidebar is large: badges will be somewhere in the middle, not aligned (because of the variable length of nicknames).

@jcbrand
Copy link
Member

jcbrand commented Aug 8, 2024

There are definitely ways to improve it. For example putting the badges and the dropdown button in a single flex container with right alignment. So they both stay on the right.

Another option is to use the Bootstrap grid system: https://getbootstrap.com/docs/5.0/layout/grid/

@JohnXLivingston JohnXLivingston marked this pull request as draft August 12, 2024 09:16
@JohnXLivingston JohnXLivingston force-pushed the occupant_action_menu branch 2 times, most recently from b6b4b9e to 280a3a6 Compare August 12, 2024 09:41
@JohnXLivingston JohnXLivingston marked this pull request as ready for review August 12, 2024 09:42
@JohnXLivingston
Copy link
Contributor Author

Ok, i think i have something that is working in all cases.
I used flex-grow so that the nickname will occupy all available spaces.
Badges will eventually go vertical if there is not enough places.

Here are some screenshots:

image

image

image

src/plugins/muc-views/templates/occupant.js Outdated Show resolved Hide resolved
src/plugins/muc-views/templates/occupant.js Show resolved Hide resolved
.occupant-actions {
// We must specify the position, else there is a bug:
// clicking on an action would close the dropdown without triggering the action.
position: static;
Copy link
Member

Choose a reason for hiding this comment

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

This looks hacky, do you know the underlying cause why this is necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No sorry, i did not understand. I spent 2 hours to find why the dropdown was not displaying correctly when using classes like dropstart (or no class at all).
I finally found that adding a position: static was correcting the issue.

Copy link
Contributor Author

@JohnXLivingston JohnXLivingston Aug 13, 2024

Choose a reason for hiding this comment

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

Hum... I just tested again without this patch... And now it is working! (position: static is the default, even without these lines... i did not have the same result yesterday...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, sorry. I'm not quite awake yet, it wasn't a display bug, but a bug on the click handler. The bug is still there when I remove these lines. And yet, position: static is the default, so it should not change anything...

@jcbrand
Copy link
Member

jcbrand commented Aug 27, 2024

@JohnXLivingston Can you please rebase and fix the conflict?

@JohnXLivingston
Copy link
Contributor Author

@JohnXLivingston Can you please rebase and fix the conflict?

Done!

@jcbrand jcbrand merged commit ae01a08 into master Aug 29, 2024
4 checks passed
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