Hi Gonzalo,<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
thanks for the patch! Here are a few notes:<br>
<br>
- when you have a text selected and want it to be spoken out a clipboard entry is created (I guess you need to have a look at why the clipboard tray reacts to that selection)<br>
<br></blockquote><div><br>I only see this problem with the Write activity, not with Read, Browse or Wikipedia-<br>May be Write is doing anything different with the clipboard?<br> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

- you specify gconf keys, those would need to be added to the schema as well<br>
<br></blockquote><div>Added.<br> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
- when you select a long text for reading there is no way to pause/stop it, might be worth having that option as well<br>
<br></blockquote><div><br>It's true. I have added controls to pause/stop in the palette. <br>I have tried with two options (please see <a href="http://wiki.sugarlabs.org/go/Talk:Features/Global_Text_To_Speech">http://wiki.sugarlabs.org/go/Talk:Features/Global_Text_To_Speech</a>)<br>
One option with menu items and the other with buttons. The option with menu items looks better,<br>but when the item is activated, the palette close, then if you want pause/play more than one time <br>is annoying. <br>The option with buttons is better for usability, because the palette is not closed,<br>
but is uglier (the separator does not take all the palette width). Advice is welcomed.<br>The code is commented in the patch then is easy test the two options  <br>(SpeechPalette __init__ and _set_buttons_state)<br><br>I don't know what to do with the hot key. Pause if the user press when a text is being played? <br>
 </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
- the shortcut is a bit long, maybe 'alt+s' is enough? (see above, maybe there is a shortcut as well for stopping? or hitting it again does stop the current playing one?)<br>
<br></blockquote><div><br>Ok, changed. To stop we use Ctrl + q<br> <br></div><div> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
- it would be great to write in the Feature page a bit more about the what the Feature does and what it does not do, after discussing with you the other day: it is available in the Shell+Activities for any text you select, it does not allow you to do activity specific operations like reading a chapter in Read, or a whole book or a wikipedia activity in Browse nor does it is a primary tool for Accessibility (no criticism intended just to note what it does and what not)<br>

<br>
Formal:<br>
<br>
- there are a few items that do not need to be public e.g. 'self.pipeline' in 'AudioGrabGst'<br>
<br></blockquote><div>I have changed them. Now I have two variables is_playing, is paused visible from outside, then added properties to access them readonly.<br> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

- the copyright in the files is a bit all over the place<br>
<br></blockquote><div>Ok. Changed. <br></div><div> <br>Thanks by the review, the patch is sent to sugar-devel.<br><br><br></div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Regards,<br>
   Simon<div class="HOEnZb"><div class="h5"><br>
______________________________<u></u>_________________<br>
Sugar-devel mailing list<br>
<a href="mailto:Sugar-devel@lists.sugarlabs.org" target="_blank">Sugar-devel@lists.sugarlabs.<u></u>org</a><br>
<a href="http://lists.sugarlabs.org/listinfo/sugar-devel" target="_blank">http://lists.sugarlabs.org/<u></u>listinfo/sugar-devel</a><br>
</div></div></blockquote></div><br>