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.

Sign up for GitHub

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

Update InfoBar narrator behavior #4578

Merged
merged 9 commits into from
Apr 22, 2021
Merged

Update InfoBar narrator behavior #4578

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7e5d976
Update infobar strings to be more descriptive and dependent on severi…
marcelwgn Mar 18, 2021
6458c6e
Add icon and announcer severity information
marcelwgn Mar 18, 2021
6c7054d
Fix bug, update wording
marcelwgn Mar 20, 2021
3be610b
Update wording
marcelwgn Mar 22, 2021
1100c62
CR feedback
marcelwgn Apr 7, 2021
3920257
Merge branch 'master' into user/chingucoding/inforbar-narrator-updates
marcelwgn Apr 16, 2021
5e10ec0
Merge branch 'master' into user/chingucoding/inforbar-narrator-updates
marcelwgn Apr 19, 2021
cb00894
Update UIA name for icon
marcelwgn Apr 19, 2021
ba81e15
Fix up indices
teaP Apr 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 65 additions & 6 deletions dev/InfoBar/InfoBar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "../ResourceHelper/Utils.h"

static constexpr wstring_view c_closeButtonName{ L"CloseButton"sv };
static constexpr wstring_view c_iconTextBlockName{ L"StandardIcon"sv };
static constexpr wstring_view c_contentRootName{ L"ContentRoot"sv };

InfoBar::InfoBar()
Expand Down Expand Up @@ -44,17 +45,23 @@ void InfoBar::OnApplyTemplate()
// Do localization for the close button
if (winrt::AutomationProperties::GetName(closeButton).empty())
{
const auto closeButtonName = ResourceAccessor::GetLocalizedStringResource(SR_InfoBarCloseButtonName);
const auto closeButtonName = ResourceAccessor::GetLocalizedStringResource(GetCloseButtonResourceName(Severity()));
winrt::AutomationProperties::SetName(closeButton, closeButtonName);
}

m_closeButton.set(closeButton);
// Setup the tooltip for the close button
auto tooltip = winrt::ToolTip();
const auto closeButtonTooltipText = ResourceAccessor::GetLocalizedStringResource(SR_InfoBarCloseButtonTooltip);
tooltip.Content(box_value(closeButtonTooltipText));
winrt::ToolTipService::SetToolTip(closeButton, tooltip);
}

if (const auto iconTextblock = GetTemplateChildT<winrt::FrameworkElement>(c_iconTextBlockName, controlProtected))
gabbybilka marked this conversation as resolved.
Show resolved Hide resolved
{
m_standardIconTextBlock.set(iconTextblock);
winrt::AutomationProperties::SetName(iconTextblock, ResourceAccessor::GetLocalizedStringResource(GetIconSeverityLevelResourceName(Severity())));
}

if (auto&& contentRootGrid = GetTemplateChildT<winrt::Button>(c_contentRootName, controlProtected))
{
winrt::AutomationProperties::SetLocalizedLandmarkType(contentRootGrid, ResourceAccessor::GetLocalizedStringResource(SR_InfoBarCustomLandmarkName));
Expand Down Expand Up @@ -161,7 +168,8 @@ void InfoBar::UpdateVisibility(bool notify, bool force)
auto const notificationString = StringUtil::FormatString(
ResourceAccessor::GetLocalizedStringResource(SR_InfoBarOpenedNotification),
Title().data(),
Message().data());
Message().data(),
ResourceAccessor::GetLocalizedStringResource(GetSeverityLevelResourceName(Severity())).data());

winrt::get_self<InfoBarAutomationPeer>(peer)->RaiseOpenedEvent(Severity(), notificationString);
}
Expand Down Expand Up @@ -193,11 +201,29 @@ void InfoBar::UpdateSeverity()

switch (Severity())
{
case winrt::InfoBarSeverity::Success: severityState = L"Success"; break;
case winrt::InfoBarSeverity::Warning: severityState = L"Warning"; break;
case winrt::InfoBarSeverity::Error: severityState = L"Error"; break;
case winrt::InfoBarSeverity::Success:
severityState = L"Success";
break;
case winrt::InfoBarSeverity::Warning:
severityState = L"Warning";
break;
case winrt::InfoBarSeverity::Error:
severityState = L"Error";
break;
};

// Do localization for the close button
if (const auto closeButton = m_closeButton.get())
{
const auto closeButtonName = ResourceAccessor::GetLocalizedStringResource(GetCloseButtonResourceName(Severity()));
winrt::AutomationProperties::SetName(closeButton, closeButtonName);
}

if (const auto iconTextblock = m_standardIconTextBlock.get())
{
winrt::AutomationProperties::SetName(iconTextblock, ResourceAccessor::GetLocalizedStringResource(GetIconSeverityLevelResourceName(Severity())));
}

winrt::VisualStateManager::GoToState(*this, severityState, false);
}

Expand Down Expand Up @@ -234,3 +260,36 @@ void InfoBar::UpdateForeground()
// If Foreground is set, then change Title and Message Foreground to match.
winrt::VisualStateManager::GoToState(*this, ReadLocalValue(winrt::Control::ForegroundProperty()) == winrt::DependencyProperty::UnsetValue() ? L"ForegroundNotSet" : L"ForegroundSet", false);
}

const winrt::hstring InfoBar::GetCloseButtonResourceName(winrt::InfoBarSeverity severity)
{
switch (severity)
{
case winrt::InfoBarSeverity::Success: return SR_InfoBarCloseButtonNameSeveritySuccess;
case winrt::InfoBarSeverity::Warning: return SR_InfoBarCloseButtonNameSeverityWarning;
case winrt::InfoBarSeverity::Error: return SR_InfoBarCloseButtonNameSeverityError;
};
return SR_InfoBarCloseButtonNameSeverityInformational;
}

const winrt::hstring InfoBar::GetSeverityLevelResourceName(winrt::InfoBarSeverity severity)
{
switch (severity)
{
case winrt::InfoBarSeverity::Success: return SR_InfoBarSeveritySuccessName;
case winrt::InfoBarSeverity::Warning: return SR_InfoBarSeverityWarningName;
case winrt::InfoBarSeverity::Error: return SR_InfoBarSeverityErrorName;
};
return SR_InfoBarSeverityInformationalName;
}

const winrt::hstring InfoBar::GetIconSeverityLevelResourceName(winrt::InfoBarSeverity severity)
{
switch (severity)
{
case winrt::InfoBarSeverity::Success: return SR_InfoBarIconSeveritySuccessName;
case winrt::InfoBarSeverity::Warning: return SR_InfoBarIconSeverityWarningName;
case winrt::InfoBarSeverity::Error: return SR_InfoBarIconSeverityErrorName;
};
return SR_InfoBarIconSeverityInformationalName;
}
6 changes: 6 additions & 0 deletions dev/InfoBar/InfoBar.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,17 @@ class InfoBar :
void UpdateCloseButton();
void UpdateForeground();

const winrt::hstring GetCloseButtonResourceName(winrt::InfoBarSeverity severity);
const winrt::hstring GetSeverityLevelResourceName(winrt::InfoBarSeverity severity);
const winrt::hstring GetIconSeverityLevelResourceName(winrt::InfoBarSeverity severity);

void OnForegroundChanged(const winrt::DependencyObject& sender, const winrt::DependencyProperty& args);

winrt::InfoBarCloseReason m_lastCloseReason{ winrt::InfoBarCloseReason::Programmatic };

winrt::Button::Click_revoker m_closeButtonClickRevoker{};
tracker_ref<winrt::Button> m_closeButton{ this };
tracker_ref<winrt::FrameworkElement> m_standardIconTextBlock{ this };

bool m_applyTemplateCalled{ false };
bool m_notifyOpen{ false };
Expand Down
3 changes: 1 addition & 2 deletions dev/InfoBar/InfoBar.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,7 @@
FontSize="{StaticResource InfoBarIconFontSize}"
Text="{StaticResource InfoBarInformationalIconGlyph}"
Foreground="{ThemeResource InfoBarInformationalSeverityIconForeground}"
FontFamily="{ThemeResource SymbolThemeFontFamily}"
AutomationProperties.AccessibilityView="Raw" />
FontFamily="{ThemeResource SymbolThemeFontFamily}"/>
</Grid>

<Viewbox x:Name="UserIconBox"
Expand Down
2 changes: 1 addition & 1 deletion dev/InfoBar/InfoBar_v1.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@
Glyph="{StaticResource InfoBarInformationalIconGlyph}"
Foreground="{ThemeResource InfoBarInformationalSeverityIconForeground}"
FontFamily="{ThemeResource SymbolThemeFontFamily}"
AutomationProperties.AccessibilityView="Raw" />
AutomationProperties.AccessibilityView="Content" />

<Border x:Name="UserIconBorder"
Grid.Column="0"
Expand Down
54 changes: 49 additions & 5 deletions dev/InfoBar/Strings/en-us/Resources.resw
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,21 @@
<resheader name="writer">
<value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
</resheader>
<data name="InfoBarCloseButtonName" xml:space="preserve">
<value>Close</value>
<comment>Automation name of the close button.</comment>
<data name="InfoBarCloseButtonNameSeverityError" xml:space="preserve">
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here and below: @gabbybilka feel free to suggest better text that should be used to announce the respective information, those were just a short text I used to plan and test things out.

<value>Error infobar close button</value>
<comment>Automation name of the close button of an InfoBar with severity error.</comment>
</data>
<data name="InfoBarCloseButtonNameSeverityInformational" xml:space="preserve">
<value>Informational infobar close button</value>
<comment>Automation name of the close button of an InfoBar with severity informational.</comment>
</data>
<data name="InfoBarCloseButtonNameSeveritySuccess" xml:space="preserve">
<value>Success infobar close button</value>
<comment>Automation name of the close button of an InfoBar with severity success.</comment>
</data>
<data name="InfoBarCloseButtonNameSeverityWarning" xml:space="preserve">
<value>Warning infobar close button</value>
<comment>Automation name of the close button of an InfoBar with severity warning.</comment>
</data>
<data name="InfoBarCloseButtonTooltip" xml:space="preserve">
<value>Close</value>
Expand All @@ -133,8 +145,40 @@
<value>InfoBar</value>
<comment>This is the custom landmark used to denote an InfoBar to narrator.</comment>
</data>
<data name="InfoBarIconSeverityErrorName" xml:space="preserve">
<value>Icon severity error</value>
Copy link
Member

Choose a reason for hiding this comment

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

@chingucoding Is it possible for the value to be the name of the icon if the icon is either the default or changed via resource from the default font set in generic.xaml?
It would be great if a user-provided icon set in IconSource had a name that we could set here as well.
@StephenLPeters has anything similar been done in other controls? Does Teaching Tip announce its icon?
@YuliKl I'm not sure of the value this announcement would provide with the current text considering the severity of the InfoBar will be announced when it opens. Maybe for when a user returns to the InfoBar after the initial announcement?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, today, icon's are entirely ignored by narrator.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@gabbybilka Current behavior, if you use the build in icons, we use our texts for UIA. If the user specify their own icon, they should be in charge of the UIA behavior of that (at least we are not interfering there to any degree).

<comment>Automation name used to announce the severity error icon.</comment>
</data>
<data name="InfoBarIconSeverityInformationalName" xml:space="preserve">
<value>Icon severity informational</value>
Copy link
Member

Choose a reason for hiding this comment

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

Can each of these icon names be updated to "{Severity} icon"?

  • "Informational icon"
  • "Success icon"
  • "Warning icon"
  • "Error icon"

Copy link
Member

Choose a reason for hiding this comment

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

@chingucoding before merging in, can you update these names?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh yes, of course. Not sure how I missed this. should be updated now.

<comment>Automation name used to announce the severity informational icon.</comment>
</data>
<data name="InfoBarIconSeveritySuccessName" xml:space="preserve">
<value>Icon severity success</value>
<comment>Automation name used to announce the severity success icon.</comment>
</data>
<data name="InfoBarIconSeverityWarningName" xml:space="preserve">
<value>Icon severity warning</value>
<comment>Automation name used to announce the severity warning icon.</comment>
</data>
<data name="InfoBarOpenedNotification" xml:space="preserve">
Copy link
Member

Choose a reason for hiding this comment

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

After testing, this string is no longer read when the InfoBar is opened. @ranjeshj as FYI but I think that is fine and preferred as the string generated by setting isDialog to true is announced instead.
In case my test was faulty, can you update the InfoBarSeverity{Severity}Name values to be "{Severity} icon" for consistency with the icon names? 😊

<value>%1!s! %2!s!</value>
<comment>The formatted string read by narrator when the InfoBar opens: "{Title} {Message}"</comment>
<value>%1!s! %2!s! %3!s!</value>
<comment>The formatted string read by narrator when the InfoBar opens: "{Title} {Message}, {severity level}"</comment>
marcelwgn marked this conversation as resolved.
Show resolved Hide resolved
</data>
<data name="InfoBarSeverityErrorName" xml:space="preserve">
<value>severity error</value>
Copy link
Member

@gabbybilka gabbybilka Mar 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@YuliKl, I think this should be "Info message, error severity" to give context that the InfoBar is a message before the content is read out. Notification could also work here?

Or just "Error InfoBar" to match the close button text.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated wording now.

<comment>Name used to announce severity error to users.</comment>
</data>
<data name="InfoBarSeverityInformationalName" xml:space="preserve">
<value>severity informational</value>
<comment>Name used to announce severity informational to users.</comment>
</data>
<data name="InfoBarSeveritySuccessName" xml:space="preserve">
<value>severity success</value>
<comment>Name used to announce severity success to users.</comment>
</data>
<data name="InfoBarSeverityWarningName" xml:space="preserve">
<value>severity warning</value>
<comment>Name used to announce severity warning to users.</comment>
</data>
</root>
13 changes: 12 additions & 1 deletion dev/ResourceHelper/ResourceAccessor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,11 +176,22 @@ class ResourceAccessor sealed
#define SR_NumberBoxDownSpinButtonName L"NumberBoxDownSpinButtonName"
#define SR_ExpanderDefaultControlName L"ExpanderDefaultControlName"

#define SR_InfoBarCloseButtonName L"InfoBarCloseButtonName"
#define SR_InfoBarOpenedNotification L"InfoBarOpenedNotification"
#define SR_InfoBarClosedNotification L"InfoBarClosedNotification"
#define SR_InfoBarCustomLandmarkName L"InfoBarCustomLandmarkName"
#define SR_InfoBarCloseButtonTooltip L"InfoBarCloseButtonTooltip"
#define SR_InfoBarCloseButtonNameSeverityInformational L"InfoBarCloseButtonNameSeverityInformational"
#define SR_InfoBarCloseButtonNameSeveritySuccess L"InfoBarCloseButtonNameSeveritySuccess"
#define SR_InfoBarCloseButtonNameSeverityWarning L"InfoBarCloseButtonNameSeverityWarning"
#define SR_InfoBarCloseButtonNameSeverityError L"InfoBarCloseButtonNameSeverityError"
#define SR_InfoBarSeverityInformationalName L"InfoBarSeverityInformationalName"
#define SR_InfoBarSeveritySuccessName L"InfoBarSeveritySuccessName"
#define SR_InfoBarSeverityWarningName L"InfoBarSeverityWarningName"
#define SR_InfoBarSeverityErrorName L"InfoBarSeverityErrorName"
#define SR_InfoBarIconSeverityInformationalName L"InfoBarIconSeverityInformationalName"
#define SR_InfoBarIconSeveritySuccessName L"InfoBarIconSeveritySuccessName"
#define SR_InfoBarIconSeverityWarningName L"InfoBarIconSeverityWarningName"
#define SR_InfoBarIconSeverityErrorName L"InfoBarIconSeverityErrorName"

#define SR_AutomationNameEllipsisBreadcrumbBarItem L"AutomationNameEllipsisBreadcrumbBarItem"

Expand Down