-
Notifications
You must be signed in to change notification settings - Fork 111
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
#971 : Media support with BuddyPress message. #1089
Conversation
app/assets/css/sass/rtmedia.scss
Outdated
@@ -39,3 +39,6 @@ | |||
|
|||
// *** RTL *** // | |||
@import "rtl"; | |||
|
|||
// *** BP Media Message *** // | |||
@import "media-message"; |
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 don't see any scss file for this @import "media-message";
if not used please remove.
app/assets/js/rtMedia.backbone.js
Outdated
@@ -497,6 +498,13 @@ jQuery( function( $ ) { | |||
}, | |||
initialize: function( config ) { | |||
this.uploader = new plupload.Uploader( config ); | |||
/* | |||
var current_url will fetch present working area's address and we will find if it contains message in url though find valirable. If message is there in URL then we need to mention that it is message. So we have appended message:true in config. |
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.
break comment into multiple lines
also follow comment structure as discussed.
app/assets/js/rtMedia.backbone.js
Outdated
var current_url will fetch present working area's address and we will find if it contains message in url though find valirable. If message is there in URL then we need to mention that it is message. So we have appended message:true in config. | ||
*/ | ||
var current_url = document.URL, find= 'message'; | ||
if(current_url.indexOf(find) !== -1){ |
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.
follow WordPress Coding Standards
app/assets/js/rtMedia.backbone.js
Outdated
if ( jQuery( '.rtm-media-msg-upload-button' ).length == 1 ) { | ||
jQuery( '.rtm-media-msg-upload-button' ).html( "" ); | ||
jQuery( '.rtm-media-msg-upload-button' ).removeAttr( "id" ); | ||
jQuery( '.rtm-media-msg-upload-button' ).html( "<p id='rtm_bpm_success' style='background: #98ef98; padding: 20px;'>Media has been attached with this message!</p>" ); |
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.
create HTML elements using jQuery.
|
||
// This function will store hidden values of Media ID in Array | ||
function store_array_in_hidden_field(){ | ||
jQuery("#rtm_bpm_uploaded_media").attr("value", msg_media_files.toString()); |
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.
jQuery("#rtm_bpm_uploaded_media").attr("value", msg_media_files.toString());
could be
jQuery( '#rtm_bpm_uploaded_media' ).attr( 'value', msg_media_files.toString() );
}); | ||
|
||
</script> | ||
<span class="primary rtm-media-msg-upload-button rtmedia-upload-media-link" id="rtm_show_upload_ui" title="Upload Media"><i class="dashicons dashicons-upload rtmicon"></i>Upload Media File</span> |
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.
break into more lines for better readability.
Add additional parameter media ID in BuddyPress message SEND MESSAGE process. and Insert data into MEDIA_META table. | ||
|
||
@param object $message Getting parameter response when sending message. | ||
**/ |
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.
comment structure
jQuery("#msg-success-bp-msg-media").hide(); | ||
jQuery(".rtm-media-msg-upload-button").attr("id", "rtm_show_upload_ui"); | ||
jQuery(".rtm-media-msg-upload-button").html(""); | ||
jQuery(".rtm-media-msg-upload-button").html("<i class='dashicons dashicons-upload rtmicon'></i>Upload Media File"); |
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.
create HTML elements using jQuery
} | ||
|
||
/** | ||
|
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.
comment structure
// Handling BuddyPress send message action by adding MEDIA ID. | ||
add_action( 'messages_message_sent', 'rtm_add_message_media_params' ); | ||
// Showing media below BuddyPress message. | ||
add_action( 'bp_after_message_content', 'show_rtm_bp_msg_media' ); |
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.
add new line at the end of file
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.
Add new line after each add_action()
call.
app/assets/js/rtMedia.backbone.js
Outdated
@@ -497,7 +498,16 @@ jQuery( function( $ ) { | |||
}, | |||
initialize: function( config ) { | |||
this.uploader = new plupload.Uploader( config ); | |||
/* | |||
/** | |||
* var current_url will fetch present working area's address and we will find if it contains message in url though find valirable. |
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.
small typo variable
app/assets/js/rtMedia.backbone.js
Outdated
if ( jQuery( '.rtm-media-msg-upload-button' ).length == 1 ) { | ||
jQuery( '.rtm-media-msg-upload-button' ).html( '' ); | ||
jQuery( '.rtm-media-msg-upload-button' ).removeAttr( 'id' ); | ||
jQuery( '.rtm-media-msg-upload-button' ).html( jQuery( '<p>', { id: 'rtm_bpm_success' }, { style: 'background: #98ef98; padding: 20px;' }, { text: 'Media has been attached with this message!' } ) ); |
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.
break into more lines for readability
app/assets/js/rtMedia.backbone.js
Outdated
if( uploaded_response_data.length<=0 ){ | ||
jQuery( '.rtm-media-msg-upload-button' ).html( '' ); | ||
jQuery( '.rtm-media-msg-upload-button' ).removeAttr( 'id' ); | ||
jQuery( '.rtm-media-msg-upload-button' ).html( jQuery( '<p>', { id: 'rtm_bpm_success' }, { style: 'background: #db001e; padding: 20px;' }, { text: 'Media attachement failed! Please try again!' } ) ); |
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.
break into more lines for readability
@@ -68,6 +68,17 @@ static function register_scripts() { | |||
$request_uri = rtm_get_server_var( 'REQUEST_URI', 'FILTER_SANITIZE_URL' ); | |||
$url = rtmedia_get_upload_url( $request_uri ); | |||
|
|||
/** | |||
* @param array $url_array | |||
* @param mixed $url |
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.
align this
jQuery(button).addClass( 'loading' ); | ||
jQuery.post( ajaxurl, { | ||
action: 'messages_send_reply', | ||
'cookie' : bp_get_cookies(), |
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.
algin this
}); | ||
return false; | ||
}; | ||
$( 'input#send_reply_button' ).unbind( 'click' ).bind('click', handler); |
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.
add spaces bind( 'click', handler );
jQuery( '#msg-success-bp-msg-media' ).hide(); | ||
jQuery( '.rtm-media-msg-upload-button' ).attr( 'id', 'rtm_show_upload_ui' ); | ||
jQuery( '.rtm-media-msg-upload-button' ).html( '' ); | ||
jQuery( '.rtm-media-msg-upload-button' ).html( jQuery( '<i>', { class: 'dashicons dashicons-upload rtmicon' }{ text: 'Upload Media File' } ) ); |
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.
break into more lines for readability
$media_url = $url[0] . '/media/' . $media_result_array_value->media_id . '/'; | ||
?> | ||
|
||
<li class='rtmedia-list-item' style='display:inline; float: left;'' id="<?php echo $media_result_array_value->media_id; // @codingStandardsIgnoreLine?>"> |
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.
don't use inline css if possible
use esc_attr
for id
style='display:inline; float: left;''
?
?> | ||
|
||
<li class='rtmedia-list-item' style='display:inline; float: left;'' id="<?php echo $media_result_array_value->media_id; // @codingStandardsIgnoreLine?>"> | ||
<a href="<?php echo esc_attr( $media_url ); ?>" class="<?php echo esc_attr( apply_filters( 'rtmedia_gallery_list_item_a_class', 'rtmedia-list-item-a' ) ); ?>"> |
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.
use esc_url
for href
and src
index.php
Outdated
@@ -107,11 +107,14 @@ function rtmedia_autoloader( $class_name ) { | |||
} | |||
} | |||
|
|||
require_once RTMEDIA_PATH.'/templates/media/media-with-message.php'; |
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.
add space before and after '.'
index.php
Outdated
/** | ||
* Register the autoloader function into spl_autoload | ||
*/ | ||
spl_autoload_register( 'rtmedia_autoloader' ); | ||
|
||
include RTMEDIA_PATH . "templates/media/media-with-message.php"; |
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.
remove include
app/assets/js/rtMedia.backbone.js
Outdated
jQuery( '<p>', { id: 'rtm_bpm_success' } ) | ||
); | ||
jQuery( '#rtm_bpm_success' ).css( {'background': '#98ef98', 'padding': '20px' } ); | ||
jQuery( '#rtm_bpm_success' ).append( 'Media has been attached with this message!' ); |
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.
localize string
app/assets/js/rtMedia.backbone.js
Outdated
@@ -795,7 +814,7 @@ jQuery( function( $ ) { | |||
return message; | |||
}; | |||
} ); | |||
|
|||
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.
remove this
app/assets/js/rtMedia.backbone.js
Outdated
uploaderObj.uploader.bind( 'FileUploaded', function( up, file, res ) { | ||
uploaderObj.uploader.bind( 'FileUploaded', function( up, file, res ) { | ||
var uploaded_response_data = JSON.parse( res.response ); | ||
if( uploaded_response_data.length<=0 ){ |
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.
add space if ( uploaded_response_data.length <= 0 ) {
app/assets/js/rtMedia.backbone.js
Outdated
{ 'background: #db001e':'#db001e', 'padding':'20px' } | ||
); | ||
jQuery( '#rtm_bpm_success' ).append( | ||
'Media attachement failed! Please try again!' |
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.
localize this
@@ -68,6 +68,17 @@ static function register_scripts() { | |||
$request_uri = rtm_get_server_var( 'REQUEST_URI', 'FILTER_SANITIZE_URL' ); | |||
$url = rtmedia_get_upload_url( $request_uri ); | |||
|
|||
/** | |||
* @param array $url_array |
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.
add comment at appropriate place
app/assets/js/rtMedia.backbone.js
Outdated
*/ | ||
var current_url = document.URL, find= 'message'; | ||
if( -1 !== current_url.indexOf( find ) ){ | ||
config.message=true; |
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.
add space config.message = true;
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.
approving PR with some minor changes
jQuery( 'form#send-reply' ).before( response ); | ||
} else { | ||
jQuery( '#message-recipients' ).after( response ); | ||
jQuery(window).scrollTop(offset.top); |
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.
jQuery( window ).scrollTop( offset.top );
$insert_media_object->insert( | ||
[ | ||
'media_id' => $media_id, | ||
// Adding meta data into custom meta table |
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.
add .
at the end of comments, sorry for not pointing out first
app/assets/js/rtMedia.backbone.js
Outdated
|
||
//current_url will fetch present URL | ||
var current_url = document.URL, find = 'message'; | ||
if( -1 !== current_url.indexOf( find ) ){ |
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.
Don't create un-necessary variables. Here find
var not used anywhere else.
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.
Please follow WordPress JS coding standards. Please follow it in your changes as of now.
app/assets/js/rtMedia.backbone.js
Outdated
if ( jQuery( '.rtm-media-msg-upload-button' ).length == 1 ) { | ||
jQuery( '.rtm-media-msg-upload-button' ).html( '' ); | ||
jQuery( '.rtm-media-msg-upload-button' ).removeAttr( 'id' ); | ||
jQuery( '.rtm-media-msg-upload-button' ).html( |
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.
Use .append()
method
app/assets/js/rtMedia.backbone.js
Outdated
if ( uploaded_response_data.length <= 0 ) { | ||
jQuery( '.rtm-media-msg-upload-button' ).html( '' ); | ||
jQuery( '.rtm-media-msg-upload-button' ).removeAttr( 'id' ); | ||
jQuery( '.rtm-media-msg-upload-button' ).html( |
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.
Use .append()
method.
app/main/RTMedia.php
Outdated
@@ -1089,6 +1089,8 @@ function enqueue_scripts_styles() { | |||
// Localizing strings for rtMedia.backbone.js | |||
$rtmedia_backbone_strings = array( | |||
'rtm_edit_file_name' => esc_html__( 'Edit File Name', 'buddypress-media' ), | |||
'rtm_bp_msg_media_success' => esc_html__( 'Media has been attached with this message!', 'buddypress-media' ), | |||
'rtm_bp_msg_media_failure' => esc_html__( 'Media attachement failed! Please try again!', 'buddypress-media' ) |
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.
Add ,
at the end of array item. PHPCS gave warning.
/** | ||
* Functionalities provided in this page will allow users to send media with BuddyPress message. | ||
* | ||
* @author malav vasita malav.vasita@rtcamp.com |
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.
@author Malav Vasita <malav.vasita@rtcamp.com>
**/ | ||
function rtm_add_message_media_params( $message ) { | ||
$insert_media_object = new RTDBModel( 'rtm_media_meta' ); | ||
$message->media_array = filter_input( INPUT_POST, 'rtm_bpm_uploaded_media' ); |
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.
You also need to pass Sanitize filters
based on data type as third argument. or you can use sanitize_text_field ()
$media_url = $url[0] . '/media/' . $media_result_array_value->media_id . '/'; | ||
?> | ||
|
||
<li class='rtmedia-list-item rtmedia_bp_msg_media_upload_render' id="<?php echo $media_result_array_value->media_id; // @codingStandardsIgnoreLine?>"> |
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.
You need to escape output using esc_attr()
and remove // @codingStandardsIgnoreLine
?> | ||
|
||
<li class='rtmedia-list-item rtmedia_bp_msg_media_upload_render' id="<?php echo $media_result_array_value->media_id; // @codingStandardsIgnoreLine?>"> | ||
<a href="<?php echo esc_url( $media_url ); ?>" class="<?php echo esc_attr( apply_filters( 'rtmedia_gallery_list_item_a_class', 'rtmedia-list-item-a' ) ); ?>"> |
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.
Don't use apply_filters()
within some function. It should be in new line with proper documentation. Save the result in a variable and then use it.
<li class='rtmedia-list-item rtmedia_bp_msg_media_upload_render' id="<?php echo $media_result_array_value->media_id; // @codingStandardsIgnoreLine?>"> | ||
<a href="<?php echo esc_url( $media_url ); ?>" class="<?php echo esc_attr( apply_filters( 'rtmedia_gallery_list_item_a_class', 'rtmedia-list-item-a' ) ); ?>"> | ||
<div class="rtmedia-item-thumbnail"> | ||
<img src="<?php echo esc_url( $media ); ?>" alt="<?php echo esc_attr( apply_filters( 'rtmc_change_alt_text', $alt_text, $rtmedia_media ) ); ?>"> |
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.
Don't use apply_filters()
within some function. It should be in new line with proper documentation. Save the result in a variable and then use it.
// Handling BuddyPress send message action by adding MEDIA ID. | ||
add_action( 'messages_message_sent', 'rtm_add_message_media_params' ); | ||
// Showing media below BuddyPress message. | ||
add_action( 'bp_after_message_content', 'show_rtm_bp_msg_media' ); |
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.
Add new line after each add_action()
call.
app/assets/js/rtMedia.backbone.js
Outdated
|
||
//current_url will fetch present URL | ||
var current_url = document.URL; | ||
if( -1 !== current_url.indexOf( 'message' ) ){ |
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.
Change it to if ( -1 !== current_url.indexOf( 'message' ) ) {
Please follow WordPress JS coding standards to keep code consistent.
app/assets/js/rtMedia.backbone.js
Outdated
jQuery( '#rtm_bpm_success' ).append( | ||
rtmedia_backbone_strings.rtm_bp_msg_media_failure | ||
); | ||
}else{ |
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.
} else {
/** | ||
* Functionalities provided in this page will allow users to send media with BuddyPress message. | ||
* | ||
* @author Malav Vasita malav.vasita@rtcamp.com |
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.
@author Malav Vasita <malav.vasita@rtcamp.com>
if ( ! empty( $media ) && null !== $media ) { | ||
foreach ( $media as $media_id ) { | ||
$insert_media_object->insert( | ||
[ |
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.
Don't use this []
array format. It won't work with PHP version <= 5.3. Use array()
syntax instead.
jQuery( '#msg-success-bp-msg-media' ).hide(); | ||
jQuery( '.rtm-media-msg-upload-button' ).attr( 'id', 'rtm_show_upload_ui' ); | ||
jQuery( '.rtm-media-msg-upload-button' ).html( '' ); | ||
jQuery( '.rtm-media-msg-upload-button' ).html( |
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.
Here also, you need to use append()
method.
$media_result = $get_data_object->get( [ 'meta_value' => bp_get_the_thread_message_id() ] ); // phpcs:ignore | ||
$url = explode( 'messages/', sanitize_text_field( wp_unslash( filter_input( INPUT_SERVER, 'REQUEST_URI' ) ) ) ); | ||
$rtm_gallary_list_filter = apply_filters( 'rtmedia_gallery_list_item_a_class', 'rtmedia-list-item-a' ); | ||
$rtm_change_alt_text_filter = apply_filters( 'rtmc_change_alt_text', $alt_text, $rtmedia_media ); |
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 told you to add inline documentation for these new filters.
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.
Filters are already there in rtmedia-filters... I have just re-used them.
So I have mention why I have used them in Function Doc Comment above.
/**
*As a result show attached media with message.
*
*@var object $get_data_object Object for having DB actions.
*@var array $media_result Getting media_ids attached with messages.
*@var array $url Partitions of current URL to redirect to perticular media.
*@var mixed $rtm_gallary_list_filter Filter for Media gallary listing.
*@var mixed $rtm_change_alt_text_filter Filter for changing alter text of an image.
**/
<div id="rtm-media-gallery-uploader" class="rtm-media-gallery-uploader"> | ||
<?php | ||
rtmedia_uploader( | ||
[ |
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.
Don't use this []
array syntax. It won't work with PHP version <= 5.3. Use array()
syntax instead.
Found some issues in this PR.
|
…ofile. Fix: Message media should not visible at user profile members/admin/media/.
c5caf45
to
cabde02
Compare
The changes in this PR are not as expected. Multiple attachment options are shown with BuddyPress Nouveau template and similarly with Legacy template. Nouveau template screenshotLegacy template screenshotAfter trying to upload the media the media appears and looks broken as well We will need to check the enhancement requested #971, get exact idea about it and then implement in my opinion. What do you suggest? cc: @thebengalboy |
We do not provide Media attachments to BuddyPress personal messages right now. We can add this functionality in rtMedia.
Issue: #971