-
Notifications
You must be signed in to change notification settings - Fork 4
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
Make links in description body clickable #104
Conversation
Nice catch, thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sold on the output escaping, maybe we can improve it
templates/event.php
Outdated
<?php echo esc_html( $event_description ); ?> | ||
<?php | ||
// phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped | ||
echo make_clickable( wp_kses_post( $event_description ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not this which would not require the override?
echo make_clickable( wp_kses_post( $event_description ) ); | |
echo wp_kses_post( make_clickable( $event_description ) ); |
wporg-gp-translation-events.php
Outdated
$title = isset( $_POST['event_title'] ) ? sanitize_text_field( wp_unslash( $_POST['event_title'] ) ) : ''; | ||
$description = isset( $_POST['event_description'] ) ? sanitize_text_field( wp_unslash( $_POST['event_description'] ) ) : ''; | ||
$title = isset( $_POST['event_title'] ) ? sanitize_text_field( wp_unslash( $_POST['event_title'] ) ) : ''; | ||
//@codingStandardsIgnoreStart |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it an option to call make_clickable()
already here? And why this style of disabling coding standards vs a phpcs:ignore
style comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After some tests, we could decide to call make_clickable()
here or in the frontend it gives the same result. Do you suggest we rather call it here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok for me now!
Fixes #76