Add new tooltip to users

[jonathanbardo]

This should close #338

@fjarrett Can you review this?

[fjarrett]

@jonathanbardo This should be $user->user_login instead.

[fjarrett]

@jonathanbardo This should also work in the Stream Records screen too.

[fjarrett]

@jonathanbardo Would be great if we could use the role label rather than slugs here. They are available in self::get_roles().

[fjarrett]

@jonathanbardo This is getting all the role labels, not the labels for this user only 😉

[fjarrett]

@jonathanbardo Also, the string concatenation here is bugging me. What do you think about something like this?

sprintf(
    __( "ID: %d\nUser: %s\nEmail: %s\nRole: %s", 'stream' ),
    $user->ID, // xss ok
    $user->user_login, // xss ok
    $user->user_email, // xss ok
    implode( ', ', $user->roles ), // xss ok
    PHP_EOL
)

[jonathanbardo]

Yeah that's good. I'll just change \n to PHP_EOL so it works across multiple system.

[westonruter]

I don't think PHP_EOL will make much difference here, since it would only matter (even slightly) for server-side processing of the textual data. But this is getting output straight to the client. So the question is, in JS do browsers like \n or \r\n more?

[jonathanbardo]

What's wrong with PHP_EOL? By using it we don't have to worry about asking
ourselves what we need to use.

On Saturday, March 15, 2014, Weston Ruter notifications@github.com wrote:

In includes/settings.php:

@@ -124,8 +121,19 @@ public static function get_users(){
}

        $args = array(

•           'id'   => $user->ID,
  

•           'text' => $user->display_name,
  

•           'id'       => $user->ID,
  

•           'text'     => $user->display_name,
  

•       );
  

•       $args['tooltip'] = esc_attr(
  

•           sprintf(
  

•               '%s%5$s%s%5$s%s%5$s%s',
  

•               __( 'ID:', 'stream' ) . ' ' . $user->ID,
  

•               __( 'User:', 'stream' ) . ' ' . $user->user_login,
  

•               __( 'Email:', 'stream' ) . ' ' . $user->user_email,
  

•               __( 'Role:', 'stream' ) . ' ' . implode( ',', self::get_roles() ),
  

I don't think PHP_EOL will make much difference here, since it would only
matter (even slightly) for server-side processing of the textual data. But
this is getting output straight to the client. So the question is, in JS do
browsers like \n or \r\n more?

Reply to this email directly or view it on GitHubhttps://github.com//pull/340/files#r10638244
.

Jonathan Bardo
Web Developer
[image: X-Team] http://x-team.com/

[westonruter]

Because we don't actually care about PHP's native EOL marker. We care about JS_EOL 😄 If on a Windows machine, PHP_EOL is \r\n then this will get sent to the browser in the code above. Whereas on a Unix machine, if PHP_EOL is \n then that will get sent to the browser. The browser doesn't care what EOL marker is native to the server. I'm just curious what differences (if any) between browsers when rendering \r\n vs \n in tooltips.

[jonathanbardo]

@fjarrett This should be good to go.

[fjarrett]

@jonathanbardo \n requires double quote style.

[fjarrett]

@jonathanbardo You may have missed my comment above that this should also apply to the Stream Records list table screen as well, not just the Settings.

[jonathanbardo]

@fjarrett I saw your comment on Stream Record but isn't that a separate plugin? You think we have to enable this here?

[fjarrett]

@jonathanbardo Not sure what you mean, Stream Records is the main screen 😸 It was part of the original requirement described in #338

[jonathanbardo]

Oh my bad... I really thought this was something else. Will take a look for this one as well.

[fjarrett]

@jonathanbardo Thanks!

[jonathanbardo]

@fjarrett Right now the record page doesn't use the same mechanism as the settings page to get all user. Do you think I can change that to be only loaded by ajax and use the same method for both functions?

[fjarrett]

@jonathanbardo there's not much here to duplicate, I'm OK with adding this separately to the list table.

[jonathanbardo]

I've added the tooltip to the record page.

[fjarrett]

@jonathanbardo Sorry but I am not seeing the title= attribute anywhere in the Stream Records screen Authors dropdown results. Maybe I'm missing something?

[fjarrett]

@jonathanbardo Sorry, I'm dumb and forgot to git pull #facepalm