search

humhub-contrib / legal

2 stars 9 forks source link

External Links Warning #50

Closed luke- closed 2 years ago

luke- commented 2 years ago

Optional Feature:

Ideally: An external link should be identified as such. Ideally, this would be done by automatically adding a corresponding icon behind the link. If not possible:

yurabakhtin commented 2 years ago

@luke- Sorry, it is not clear where the external links can be defined in this module legal. Maybe you mean to search links inside the content before print it, i.e. we find all links inside content and if it is an external then add to the link a confirmation dialog.

But you wrote about "check after the user clicks", it seems you want to check somehow by JS or AJAX?

luke- commented 2 years ago

@yurabakhtin The wording "check after click" is a bit misleading, especially after we ideally want to display an icon/symbol behind it beforehand as well.

yurabakhtin commented 2 years ago

@luke- PR https://github.com/humhub-contrib/legal/pull/53:

external-links

luke- commented 2 years ago

@Semir1212 Can you please check the Wording?

Semir1212 commented 2 years ago

@yurabakhtin

External Link

This link leads to an external website. Would you like to proceed?

Cancel // Proceed

yurabakhtin commented 2 years ago

@luke- @Semir1212 Commit https://github.com/humhub-contrib/legal/pull/53/commits/b987ed3300f7097a0ec9cc0a5af9206516987a6f:

wording

luke- commented 2 years ago

@yurabakhtin it already looks good. Can you please make this feature optional and extend it to posts/comments. image image

yurabakhtin commented 2 years ago

@luke- The content of legal page was rendered by RichText::output($content). I have extended this only for the module legal.

If we need the same for posts and comments, does it mean I should move the new code from the legal into core in order to extend the RichText widget with options like RichText::output($content, ['externalLinks' => [...]]), and then remove the duplicated code the legal module?

If yes, what core branch should be updated, develop?

luke- commented 2 years ago

@yurabakhtin Couldn't this also be solved via Javascript?

yurabakhtin commented 2 years ago

@luke- Yes, the markdown of the richtext is rendered really by JS, so my current solution was done here https://github.com/humhub-contrib/legal/pull/53/files#diff-124ba9b0f8c3920fc7d0e3202f8627a95c2ad97b5591702c56fc43ce20c4af98 by JS, so if I will move this solution in the core then it will be moved here https://github.com/humhub/humhub/blob/master/protected/humhub/modules/content/resources/js/humhub.ui.richtext.prosemirror.js#L155 or some additional file that will extends the JS widget RichText.

luke- commented 2 years ago

I don't quite understand that right now, what exactly do you want to transfer into the Core? The external link marker?

yurabakhtin commented 2 years ago

@luke- If you need the same feature from current PR should be applied to core posts/comments then almost all JS code from the file resources/js/humhub.legal.js - https://github.com/humhub-contrib/legal/pull/53/files#diff-124ba9b0f8c3920fc7d0e3202f8627a95c2ad97b5591702c56fc43ce20c4af98 should be moved to core.

luke- commented 2 years ago

@yurabakhtin Hmm, I actually don't like that if such module logic comes into the core.

What is the reason why we can't leave this external?

The module could just inject its javascript on each page and work on the OnReady/PjaxReady events?

yurabakhtin commented 2 years ago

@luke- Ah now I got you, we need to inject it for posts and comments from the module legal, ok I will implement this.

luke- commented 2 years ago

Ok, so there should be no changes from the core.

yurabakhtin commented 2 years ago

@luke- New changes have been done:

post_comment_external_links

I think we should inform the checkboxes are applied also for content of Posts and Comments.

luke- commented 2 years ago

@yurabakhtin Thanks, I'll add the text info. Maybe we need to expand the external links info also for other RichText in future.