Skip to content
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

Improve UX in error messages (#1568) #1732

Merged
merged 14 commits into from
Jun 21, 2021
Merged

Conversation

ann0see
Copy link
Member

@ann0see ann0see commented May 21, 2021

This is the a start for a discussion!

I looked over some error messages especially concerning the sound system and changed some. It would be great if @gilgongo and @mulyaj looked over these messages from an UX point of view. Also I'd appreciate if @pljones or @softins looked at the content.

Context:
#1568

Note: I need to keep jamulussoftware/jamuluswebsite#339 in mind.

Missing before merge:

Final review

@ann0see ann0see changed the title mprove UX in error messages (#1568) Improve UX in error messages (#1568) May 21, 2021
linux/sound.cpp Outdated Show resolved Hide resolved
linux/sound.cpp Outdated Show resolved Hide resolved
@gilgongo
Copy link
Member

I should add the ritual UX invocation on all this: before sitting down to write an error message, consider whether the system can be changed such that the message isn't needed :-)

linux/sound.cpp Outdated Show resolved Hide resolved
linux/sound.cpp Outdated Show resolved Hide resolved
mac/sound.cpp Outdated Show resolved Hide resolved
mac/sound.cpp Outdated Show resolved Hide resolved
src/soundbase.cpp Outdated Show resolved Hide resolved
src/soundbase.cpp Outdated Show resolved Hide resolved
src/soundbase.cpp Outdated Show resolved Hide resolved
src/soundbase.cpp Outdated Show resolved Hide resolved
windows/sound.cpp Outdated Show resolved Hide resolved
@mulyaj
Copy link
Contributor

mulyaj commented May 21, 2021

To add, I think contractions should be kept consistent. Currently, there's some variance for doesn't and couldn't.

In my opinion, I think we should use the contracted form to sound a bit less robotic.

@hoffie hoffie added this to the Release 3.8.1 milestone May 21, 2021
@ann0see
Copy link
Member Author

ann0see commented May 21, 2021

In my opinion, I think we should use the contracted form to sound a bit less robotic.

Agree. This comes with the unification of style&tone probably?

@ann0see
Copy link
Member Author

ann0see commented May 21, 2021

In my opinion, I think we should use the contracted form to sound a bit less robotic.

Yes. Probably that's a good solution. Let's wait for others to comment.

Note to maintainers: Good candidate for squash and merge. Will maybe do it manually

Copy link
Member Author

@ann0see ann0see left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went through it again...

mac/sound.cpp Outdated Show resolved Hide resolved
mac/sound.cpp Show resolved Hide resolved
mac/sound.cpp Show resolved Hide resolved
mac/sound.cpp Show resolved Hide resolved
mac/sound.cpp Outdated
@@ -463,7 +463,7 @@ QString CSound::CheckDeviceCapabilities ( const int iDriverIdx )
( !( CurDevStreamFormat.mFormatFlags & kAudioFormatFlagIsPacked ) ) )
{
return tr ( "The audio input stream format for this audio device is "
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return tr ( "The audio input stream format for this audio device is "
return tr ( "The audio input stream format for this sound hardware is "

This suggestion sounds strange to me but would be needed if we follow the style guide.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the trouble with style guide vocabulary rules is that don't always fit the context. I'd say it would read a bit better if it was ".. for your sound hardware" perhaps.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The stream format on the current input device is not %1. Please select another device or reconfigure.

I really don't like hardware. From Jamulus's perspective, it's just the audio source or audio sink. We have no idea what's on the end of the device driver.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could maybe say in the guidelines that "device" is interchangeable with "hardware" if needed? The former is less newbie friendly, so would prefer "hardware" in most cases, but here perhaps "device" is more helpful maybe the OS refers to "devices" too?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Windows, click the speaker icon in the tooltray to pop open the the volume, then click the "^" to pop up the selector list -- it says "Select playback device". So Windows users will understand "recording device" (for an input) and "playback device" (for an output).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The overall control is "Open Sound settings", where "Output"->"Choose your output device" and "Input"->"Choose your input device" live.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we could modify what we have in the guide so that "device" would be OK along with related terms:

"Device" to refer to audio hardware seen by the OS. "No device selected", "Input device”, "Choose the device".

“Audio interface” to refer to external devices. “If your audio interface comes with a driver”, “Your audio interface may be set to ‘monitor’”.

"Sound card" to refer to built-in audio devices. "Your computer's sound card".

“Sound hardware” refers generically to both internal and external devices. “Your sound hardware will introduce some latency”, “Most sound hardware can be used”.

Copy link
Collaborator

@pljones pljones May 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, this is mac/sound.cpp, so it should follow Apple MacOS conventions, rather than Microsoft Windows conventions. We should follow OS language conventions in OS-specific messages. JACK has its own naming conventions, too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. So "stick to naming conventions your OS uses" should be added to the google doc probably?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mulyaj to discuss this one today

windows/sound.cpp Show resolved Hide resolved
windows/sound.cpp Show resolved Hide resolved
windows/sound.cpp Outdated Show resolved Hide resolved
windows/sound.cpp Show resolved Hide resolved
windows/sound.cpp Show resolved Hide resolved
windows/sound.cpp Outdated Show resolved Hide resolved
linux/sound.cpp Outdated Show resolved Hide resolved
linux/sound.cpp Outdated Show resolved Hide resolved
linux/sound.cpp Outdated Show resolved Hide resolved
linux/sound.cpp Outdated Show resolved Hide resolved
mac/sound.cpp Outdated Show resolved Hide resolved
mac/sound.cpp Outdated Show resolved Hide resolved
mac/sound.cpp Outdated Show resolved Hide resolved
src/soundbase.cpp Outdated Show resolved Hide resolved
src/soundbase.cpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@pljones pljones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the basis there are several planned follow ups...

@ann0see
Copy link
Member Author

ann0see commented Jun 10, 2021

On the basis there are several planned follow ups...

So probably

  1. An agreement on the audio device wording
  2. JACK as variable
  3. Removal of clean up (can’t guarantee that)
  4. What’s missing?

@pljones
Copy link
Collaborator

pljones commented Jun 11, 2021

1 & 2 are all I could remember without going through all the closed off review comments again.

linux/sound.cpp Outdated Show resolved Hide resolved
linux/sound.cpp Outdated Show resolved Hide resolved
linux/sound.cpp Outdated Show resolved Hide resolved
ann0see and others added 3 commits June 11, 2021 22:49
Co-authored-by: Martin Passing <github@passing.name>
Co-authored-by: Martin Passing <github@passing.name>
Co-authored-by: Martin Passing <github@passing.name>
@ann0see
Copy link
Member Author

ann0see commented Jun 19, 2021

@gilgongo could you please review the code so that we can squash and merge this?

linux/sound.cpp Outdated Show resolved Hide resolved
linux/sound.cpp Outdated Show resolved Hide resolved
linux/sound.cpp Outdated Show resolved Hide resolved
linux/sound.cpp Outdated Show resolved Hide resolved
linux/sound.cpp Outdated Show resolved Hide resolved
src/soundbase.cpp Outdated Show resolved Hide resolved
windows/sound.cpp Outdated Show resolved Hide resolved
APP_NAME + tr ( " software." );
return QString ( tr ( "The current audio device configuration is incompatible "
"because the sample rate couldn't be set to %1 Hz. Please check for a hardware switch or "
"driver setting to set the sample rate manually and restart the app." ) )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does "the app" mean here? If it's Jamulus, we should say that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Member

@gilgongo gilgongo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably need to use more active case in the English here, for clarity.

@ann0see ann0see merged commit 40d4b47 into jamulussoftware:master Jun 21, 2021
@ann0see ann0see deleted the ChangeMSG branch June 21, 2021 18:47
@ann0see
Copy link
Member Author

ann0see commented Jun 21, 2021

CHANGELOG: Rewrote multiple error messages to improve UX.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants