Feedback on Discussants

hgtonight

I really like this! You did a great job commenting your code.

The only issue I ran in to is that the hidden avatars aren't hidden automatically, but do once you hove over any of them (and subsequent move your cursor off of them). This was on Firefox 21.0 on Win 7.

vrijvlinder

This is cool ! it's like in this conversation but on the discussions

candyman

It would be nice to add the number of the discussants beside the number of comments.

R_J

Thank you all! I have fixed this little annoyance and added a second custom.css you can try. Just to show that it is easily changeable how the information is transported.

@candyman: I'll have a look at it. Seems as if this couldn't be too hard to code on the html side. I'll just have to look where to hook and how to style this. And I agree to you: for me this is a sign how lively a discussion is and so it is a nice information to have

candyman

Thank you for your attention, great plugin BTW.

R_J

MUAHAHA! Adding the "X discussant(s)" text was ridiculously easy! I'll have to update the screenshots before I can post the new file in plugins, but if you like to try it beforehand, you can add this code to class.discussants.plugin.php

    // Add view for text: X discussant(s)
    public function CategoriesController_AfterCountMeta_Handler($Sender) {
      $this->DiscussantsCountView($Sender);
    }
    public function DiscussionsController_AfterCountMeta_Handler($Sender) {
      $this->DiscussantsCountView($Sender);
    }
    public function ProfileController_AfterCountMeta_Handler($Sender) {
      $this->DiscussantsCountView($Sender);
    }    
    private function DiscussantsCountView($Sender) {
         // get Discussants of current Discussion and exit if something went wrog
        $Discussants = unserialize(GetValue('Discussants', $Sender->EventArguments['Discussion']));
        if ($Discussants==false) {
            return;
        }
        // get number of discussants
        $DisCount = count($Discussants[1]);
        $output = '<span class="DiscussantCount">'.$DisCount.' discussant';
        if ($DisCount > 1) {
            // use plural if more than one discussant
            $output  .= 's';
        }
        $output .= '</span>';
        echo $output;
    } // End of DiscussantsCountView

Thanks to you @candyman for the inspiration

candyman

Great. The problem is when you try to localize it.
Adding the 's' isn't working with Italian, for example.
We need two different words:

partecipante (1) and partecipanti (plural) alla discussione (that can be omitted).

It would be better two different words in the code in order to translate them in a simple way.

hgtonight

@R_J I am just going to leave this here:

 // get number of discussants
$DisCount = count($Discussants[1]);
$output = Wrap(
  Plural($DisCount, '1 discussant', '%s discussants'),
  'span',
  array('class' => 'DiscussantCount')
);

The Plural function takes a number, and two strings. If the number is 1, it will pass the first string through the T() function. If it is any other number, it will pass the second string through the T() function. In both cases, it will insert the number in %s if it exists in the definition.

R_J

Thanks for the feedback! :-)

I've updated the sources on github already. Here's the new code for the DiscussantsCountView any way. I'll upload a new zip in the addon section later on.

    private function DiscussantsCountView($Sender) {
         // get Discussants of current Discussion and exit if something went wrog
        $Discussants = unserialize(GetValue('Discussants', $Sender->EventArguments['Discussion']));
        if ($Discussants==false) {
            return;
        }
        // get number of discussants
        $DisCount = count($Discussants[1]);
        $output = '<span class="DiscussantCount">'.$DisCount.' ';
        if ($DisCount > 1) {
            // use plural if more than one discussant
            $output  .= T('discussants');
        } else {
            // or singular for one discussant
            $output  .= T('discussant');
        }
        $output .= '</span>';
        echo $output;
    } // End of DiscussantsCountView

Doing updates this way saves me from running out of version numbers below 1.0 :-D

R_J

@hgtonight: sorry, I haven't seen your posting. Your code looks a lot more elegant than my version!

And it's really nice to see that Garden has such useful supporting functions. I already knew about Wrap, but didn't found it useful, but I may think it over. In this case it fits perfect.
Plural looks like a great idea! I suppose it could be used in any plugin with text output :-)

I'll update github asap

hgtonight

I will be the first to admit I really like Wrap().

Garden has excellent, built-in functions. Check out /library/core/functions.* for others that are available as long as the framework is loaded.

candyman

@R_J said:
Plural looks like a great idea! I suppose it could be used in any plugin with text output :-)

Maybe a stupid question (I'm not a coder at all): isn't possible to use a default string, in this case the word discussion(s), and to add a localization page for the plugin in order to modify in a "for dummies" way, the one according the user needs (language).
So the code (and its revisions) remains the same and the user can choose between .it, .es. .us localizations or create their own that translate that string.

Actually I have to search the code and change the string I have to translate. Not a great job but the possibility for the user to easy translate&customize the words is a thing that the developer should consider.
Maybe in the Garden there is something to do this?

R_J

@candyman: the Vanilla way is to use a locale folder. I'll add one to the new upload.
Look at the subdirectory "locale". There you can put translations. I'll try one for Italian. Just be patient, but it will come in the next 20-30 minutes

R_J

@candyman: done! If you're interested in the technical aspects of localization, please look here. There is one very important sentence on this side: "Clear the .ini files in your /cache folder after adding a new translation file."

But there is more than one .ini file there. Delete /cache/locale_map.ini and you see the text for "it-IT" (I suppose that is the language you have chosen in your dashboard under "Locales" as "Default Locale").

Take a look in the plugin at the file "Discussants/locale/it-IT.php" to change the text to any text you want to have.

candyman

Thanks!

hgtonight

I think this needs an icon. Maybe @vrijvlinder can help?

vrijvlinder

yes it seems this one slipped , I will come up with something ...

vrijvlinder

@R_J

http://vanillaforums.org/discussion/24612/icon#latest