Added social media links to peer reviewed article #244
No reviewers
Labels
No labels
Component: User interface
Component: Wymeditor
Help wanted
Level: Difficult
Level: Easy
Level: Moderate
Pagetype: Festival
Pagetype: Mailinglist
Pagetype: Peer reviewed article
Pagetype: Text
Privacy GDPR AVG
status: has conflicts
Status: Needs changes
Status: Needs discussion
Status: Needs review
Status: Ready to merge
Status: Waiting for response
Type: Bug
Type: Enhancement
Type: Question
Usecase: De Stadsbron
Usecase: Koppelting
Usecase: MeetjeStad
Value: Coders
Value: Security
Value: Users
Value: Visitors
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
harmen/hypha!244
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "medialinks"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Possible fix for #240 ?
Needs CSS implementation. @tammoterhark: images are now hardcoded to be looked for in the data/images/ folder. Probably you want this to be arranged for in a theme folder.
This feature should probably be implemented as an abstract function (something like 'addMediaLinks') that would be available to all page types. For now I propose to send that to the technical debt department and start testing this feature in peer reviewed article.
Please make this also switchable from the admin settings: for The Stadsbron this is now needed, but not for other Hypha instances.
No, not a fix for #240, since that is about posting to the own social media by editors of De Stadsbron.
The solution that you implemented is for readers of the Stadsbron.
What do you mean by switchable from admin?
I mean that in the admin interface (not now, but needed when other parties will want to use this page type) this feature needs to have the possibility to be switched off. Maybe later: switch off/on per page type?
I've added some comments about encoding. Additionally, the resulting href values should also be htmlspecialchar'd. The result of urlencode should already be safe enough, but there are also some
&in the href values that provide technically incorrect HTML.Maybe some of the new form interpolate stuff and DomDocument object-building might be easier to fix this.
I think the subject and body should be urlencoded here.
I think the url= value should be urlencoded.
I think the the u= value should be urlencoded here.
I think this is a good idea, but I'm not entirely sure how fine-grained this should be. Should you then offer options to enable facebook/twitter/email separately? Should we have options for other datatypes too? Maybe just add this unconditionally for now, and then once we decide how to integrate this in other datatypes too, then decide how we make this optional as well.
Also, this probably interactions with #258, so I think we should merge that and afterwards I can have a look at updating and finishing up this PR.
I've updated this PR to:
There's two more things to be done:
For the last point:
I've put the code of this PR on test.destadsbron.nl, so it can be tested and styled there.
@tammoterhark, can you pick up the above points?
I Have added the icons via CSS, so they can be changed by the designer more easily.
But I also found that the Facebook link gave an error message: Parameter 'href' should represent a valid URL
I believe that is because you are testing using a localhost url, which Facebook does not consider valid (it would not make sense to share in any case).
I tested on test.destadsbron.nl, works as expected there.
CSS looks good. I've rebased on top of master and will merge in a minute.