GNOME Bugzilla – Bug 695635
No documentation exists for audible notifications/sounds.
Last modified: 2018-05-22 16:02:34 UTC
Compiled from git master, empathy does not seem to have any documentation about audible notifications and how to enable/disable sounds.
Thanks for this bug report. I will upload the patch soon.
Created attachment 242069 [details] [review] Help page to enable or disable sound notifications The patch explains: Default settings used for sound notifications How to change the default settings Enable or disable sound notification during various events
Review of attachment 242069 [details] [review]: Hi, thanks for the patch Sowmya. Have you had a look at http://projectmallard.org ? It has some really nice examples on how to use Mallard markup well. If you have not looked at it yet, you may want to start with http://projectmallard.org/about/learn/tenminutes.html and then use http://projectmallard.org/1.0/ for reference as you work. Make sure to reflow your paragraphs to 79 characters long, I will give you an example later on in the review of what I mean. ::: help/C/sound-notifications.page @@ +1,3 @@ +<page xmlns="http://projectmallard.org/1.0/" + xmlns:its="http://www.w3.org/2005/11/its" + xmlns:if="http://projectmallard.org/if/1.0/" Why are you using xmlns:its and xmlns:if? You don't actually use either one in the page. As it is, you should use xmlns:its to markup your email address as untranslatable (and your name if you do not want translators to transliterate it). @@ +7,3 @@ + <info> + <link type="guide" xref="index"/> + <desc> Sound notifications are enabled to notify the users when they receive a chat </desc> There is an extra space at the start of the description and another one at the end. Descriptions are supposed to be sentences, so use a full stop at the end. (I think this may have been mentioned in the review of your other patch.) One does not receive chats, one receives a message. @@ +11,3 @@ + <name>GNOME Documentation Project</name> + <email>gnome-doc-list@gnome.org</email> + </credit> I'm pretty sure that this isn't a person, so remove it. @@ +19,3 @@ + </info> + + <title> Enable or disable sound notifications</title> Again, extra space at the start. Please be more careful in general. @@ +20,3 @@ + + <title> Enable or disable sound notifications</title> + <p>Sound notifications are used to notify the user with alert sound during various events such as when a user receives a chat</p> Sentences end in full stops. Your explanation of sound notification repeats itself; it's better if you don't try to explain that they are notifications that produce a sound unless you can think of a better way to explain it. This is what the paragraph should look like when reflowed to 79 characters long: <p>Sound notifications are used to notify the user with alert sound during various events such as when a user receives a chat</p> @@ +23,3 @@ + <p>The default settings are: + Sound notification is enabled when you are online + Sound notification is disabled when you are away or busy</p> Lists need to be in <list> elements. See projectmallard.org for more details. @@ +25,3 @@ + Sound notification is disabled when you are away or busy</p> + + <section id="change-default-settings"> It's better to keep section IDs short. For example, if this ID was "edit", then it would be written out as "sound-notification#edit" when being linked to (which is the whole point of having IDs), whereas right now, it is "sound-notification#change-default-settings" which is a bit of a mouthfull. @@ +28,3 @@ + <title>Change the default settings</title> + <p>You can always change the default settings. + To change:</p> At the very least, this should be indented by four spaces, not six, but don't break the paragraph by the sentence. @@ +37,3 @@ + Disable sounds when away or busy</p> + </item> + </steps> Read on projectmallard.org how to use step lists. @@ +40,3 @@ + + <p>In the sound section, you can find the subsection “play sounds for events”. + This provides options to enable or disable sound notification during various events such as: Mark up the subsection properly using <gui> tags. Don't break the line based on sentences, don't use funky indentation. @@ +49,3 @@ + Account connected + Account disconnected + </list> Read on projectmallard.org how to use lists.
Created attachment 242086 [details] [review] Help page to enable or disable sound notifications modified Thanks a lot Kat for your comments. It was a good learning experience. I have modified the patch Please review
Review of attachment 242086 [details] [review]: Have you validated the page yet? You can do this using yelp-check, which comes as part of yelp-tools. ::: help/C/sound-notifications.page @@ +4,3 @@ + <info> + <link type="guide" xref="index"/> + <desc>Sound notifications are enabled to notify the users when they receive a message.</desc> This help is directly for the user, not someone setting this up on behalf of the user. @@ +14,3 @@ + <title>Enable or disable sound notifications</title> + <p>Sound notifications are used to notify the user during various events such as when a user receives a message + . The default settings are</p> There should not be a space between the end of a sentence and the full stop. Reflow to 79 characters wide. It makes sense to have a colon at the end, before the list. @@ +24,3 @@ + <p>You can always change the default settings. To change</p> + <steps> + <item><p>Click on edit tab and then open preferences tab.</p></item> Are you using the latest version of GNOME? These instructions are incorrect as of 3.6.4, so are not correct for master either. Try a newer version of Empathy to check the correct instructions. Instructions on how to access something in a menu. These should be marked up using a <guiseq>, read about how to use these on http://projectmallard.org/1.0/. @@ +25,3 @@ + <steps> + <item><p>Click on edit tab and then open preferences tab.</p></item> + <item><p>Go to Sounds section.</p></item> Sounds is a tab, this should be marked up as <gui>. @@ +26,3 @@ + <item><p>Click on edit tab and then open preferences tab.</p></item> + <item><p>Go to Sounds section.</p></item> + <item><p>You can check/uncheck the checkbox corresponding to the the following Reword so that you don't use a slash. @@ +31,3 @@ + <list> + <item><p>Enable sound notifications.</p></item> + <item><p>Disable sounds when away or busy.</p></item> You may want to explain what these mean in more detail. Like what happens when they are checked, for example. @@ +33,3 @@ + <item><p>Disable sounds when away or busy.</p></item> + </list> + <p>In the sound section, you can find the subsection<gui>play sounds for events You're missing a space between "subsection" and <gui>. <gui> elements should use the same capitalisation as is used in the UI. @@ +34,3 @@ + </list> + <p>In the sound section, you can find the subsection<gui>play sounds for events + </gui>. This provides options to enable or disable sound notification during <gui> elements need to be closed on the same line as the last word of the element content. @@ +44,3 @@ + <item><p>Account connected</p></item> + <item><p>Account disconnected</p></item> + </list> If you list these, you should explain what they mean, otherwise what is your reasoning for listing them?
Created attachment 270702 [details] [review] A new help page is added for audible notifications Empathy did not have documentation about audible notifications. This help page has information about default settings used for sound notifications, steps to change them and how to enable or disable sound notification during various events
Comment on attachment 270702 [details] [review] A new help page is added for audible notifications >Subject: [PATCH] A new help page is added for audible notifications Personal preference: Could you change the passive construction "Subject: [PATCH] A new help page is added for audible notifications" to "Add page to describe audible notifications"? > help/C/legal.xml | 5 +++ I'd prefer to have the addition of "help/C/legal.xml" in a separate patch - it's not clear from the patch summary that your patch also introduces this file. >+ <item><p>Select <guiseq><gui style="menu">Perferences</gui><gui>Sounds Small typo, should be Preferences >+ <item><p>You can check the checkbox corresponding to the following two >+ options:</p> Meh, "check the checkbox"? :-/ Plus if there are two options, they are checkboxES I'd guess? >+ <item> >+ <title>Enable sound notifications.</title> >+ <p>Sound will be enabled for the selected events from <gui>Play sound >+ for events.</gui></p> I haven't checked out the Empathy UI, but from just reading this I don't understand what "Sound will be enabled for the selected events from Play sound for events" means. What selected events? What does "from Play sounds for events" mean here? I don't understand the sentence structure. Is the fullstop really part of the UI element? >+ <section id="second"> >+ <title>Select events from <gui>Play sound for events:</gui></title> Is the colon really part of the UI element? >+ <p>Sound will be enabled for the selected events:</p> If that is the case and if I cannot change anything anyway according to your "describing" documentation (at least I don't see any action mentioned that I could take), does it make sense / what is the intention to list all these items here?
>+ <section id="second"> As id's might be used as anchors in HTML links ( http://example.com/foo.html.en#bar ) I don't think that "#second" is very descriptive.
Created attachment 270840 [details] [review] Add page to describe audible notifications Empathy did not have documentation about audible notifications. This help page has information about default settings used for sound notifications, steps to change them and how to enable or disable sound notification during various events
Hmm, did you mistakenly attach the same older patch again? The first two items of my review in comment 7 also apply to the patch in comment 9.
Created attachment 270857 [details] [review] Add page to describe audible notifications Empathy did not have documentation about audible notifications. This help page has information about default settings used for sound notifications, steps to change them and how to enable or disable sound notification during various events
Created attachment 270860 [details] [review] Add page to describe audible notifications Empathy did not have documentation about audible notifications. This help page has information about default settings used for sound notifications, steps to change them and how to enable or disable sound notification during various events
Comment on attachment 270860 [details] [review] Add page to describe audible notifications >+ <p>Sound will be disbled when the user is away or busy.</p> Typo: disbled >+ <title>Select events in <gui>Play sound for events</gui>:</title> The <title> tags above either used no punctuation at the end, or a full stop. Now there is a colon used here. Could you explain when you use which? :) >+ <p>You can enable sound for the below mentioned events by checking the >+ checkboxes against them:</p> I'm not happy with "checking the checkboxes against them" and wonder if "checking the corresponding checkboxes" would be sufficient, but would love to have another opinion here. >+ <item><title>Contacts comes online</title> The string in #: ../src/empathy-preferences.c:141 is "Contact comes online"
Created attachment 272212 [details] [review] Help page added to describe audible notifications Empathy did not have documentation about audible notifications. This help page has information about default settings used for sound notifications, steps to change them and how to enable or disable sound notification during various events
Review of attachment 272212 [details] [review]: ::: help/C/sound-notifications.page @@ +1,2 @@ +<page xmlns="http://projectmallard.org/1.0/" + xmlns:e="http://projectmallard.org/experimental/" Where do you use the experimental namespace? @@ +3,3 @@ + type="topic" style="task" + id="sound-notifications"> + <info> Blank line above this please. @@ +17,3 @@ + <p>Creative Commons Share Alike 3.0</p> + </license> + </info> Check the style that is used in epiphany and follow it for the <info> tag. @@ +19,3 @@ + </info> + <title>Enable or disable sound notifications</title> + <p>Sound notifications are used to notify the user during various events notify the user *of* @@ +20,3 @@ + <title>Enable or disable sound notifications</title> + <p>Sound notifications are used to notify the user during various events + such as when a user receives a message. The default settings are:</p> Defaults change, lets not list them. @@ +27,3 @@ + </list> + <section id="default"> + <title>Change the default settings</title> You should review what you're trying to achieve in this page. Changing the default setting is not a particularly worthwhile goal in itself. Good goals are, for example, to disable all sound notifications or to enable sound notifications when one is away/busy. @@ +28,3 @@ + <section id="default"> + <title>Change the default settings</title> + <p>To change the default settings:</p> Don't repeat yourself, use different wording. @@ +31,3 @@ + <steps> + <item><p>Select <guiseq><gui style="menu">Preferences</gui><gui>Sounds + </gui></guiseq>.</p></item> Close tags in the same line as their content. The preferred style for indentation is: <steps> <item> <p></p> </item> </steps> @@ +67,3 @@ + <item><title>Account disconnected</title> + <p>Sound is played on disconnection of account.</p></item> + </list> I think you were aiming to have a <terms> list here, but I'm not sure if a list is needed at all as it seems to recite the UI. ::: help/Makefile.am @@ +61,3 @@ set-custom-status.page \ share-desktop.page \ + sound-notifications.page \ Good :)
Patch looks good to me; leaving it to Kat if it makes sense to really list the options that are already in the UI, as it feels a bit like "Click Save to save" style to me (documentation that is not necessarily helpful for the users).
Created attachment 273551 [details] [review] This help page explains how to disable all sound notifications, enable it when user is away or busy and about enabling it for various events. Updated patch according to the reviews. Please review again, thanks.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/empathy/issues/659.