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

Enh #257: Add "Send message" to People cards #258

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

Conversation

marc-farre
Copy link
Contributor

See problem here: humhub/humhub#5622

'unfollowOptions' => ['class' => 'btn btn-primary btn-sm active'],
]);

$html .= FriendshipButton::widget([

Choose a reason for hiding this comment

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

This should have an if tag in cases where friendship is disabled.

@marc-farre
Copy link
Contributor Author

marc-farre commented Mar 31, 2022

@ArchBlood this is not necessary as the FriendshipButton already checks if the module is enabled. And if not, it returns an empty string. It is the same as in https://github.com/humhub/humhub/blob/develop/protected/humhub/modules/user/widgets/PeopleActionButtons.php#L44

@luke-
Copy link
Contributor

luke- commented Mar 31, 2022

@funkycram Thanks for the contribution. I would not like to work with the "CREATE" event in the mail module and replace the original button.

Instead, it would be nicer to create a PeopleActionButtons widget based on the BaseStack widget in the HumHub Core repository at the Buttons. Then we could simply inject the button via the mail module.

What do you think about it?

@luke- luke- linked an issue Mar 31, 2022 that may be closed by this pull request
@marc-farre
Copy link
Contributor Author

@luke- sorry, I'm not sure I understand. Is it the "Describe alternatives you've considered" solution I described in humhub/humhub#5622 ?

@luke-
Copy link
Contributor

luke- commented Apr 1, 2022

@funkycram Sorry, I hadn't read the source code correctly. Your approach is ok.

However, we can't show "Follow" + "Friendship" + "Send message" because the space would be too limited.

What do you think about replacing either Follow or Friendship?

I have changed the class here in the core a bit for this to be easier.

humhub/humhub@fccd93c

@marc-farre
Copy link
Contributor Author

Thanks @luke- .
See 9846a6f

I think we could keep the 3 buttons:
image

What do you think about the image icon which makes the button smaller?

public function run()
{
$html = $this->addFollowButton();
$html .= $this->addFriendshipButton();
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest that we replace this button/method with "message" if you are friends. What do you think about that?

3 buttons are too much and probably look too much or?
Can you post a screenshot of it?

Choose a reason for hiding this comment

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

I would suggest that we replace this button/method with "message" if you are friends. What do you think about that?

3 buttons are too much and probably look too much or?
Can you post a screenshot of it?

An alternative would be using icons over basic text based buttons, better control over the size.

@luke- luke- assigned Eladnarlea and unassigned Eladnarlea Mar 13, 2023
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.

Add "Send message" to People cards
4 participants