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

Jump to bottom

AppNotificationBuilder - Code Improvements #2801

Merged
merged 5 commits into from
Aug 2, 2022
Merged

AppNotificationBuilder - Code Improvements #2801

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ae443e2
Addressing nits from a previous PR
Jul 29, 2022
c23273c
Uniform naming for tests
Jul 30, 2022
d737678
Addressing PR feedback
Aug 1, 2022
54363e7
IsWindows... is the standard method for checking Windows version
Aug 1, 2022
62a7247
nit
Aug 1, 2022
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
64 changes: 32 additions & 32 deletions dev/AppNotifications/AppNotificationBuilder/AppNotificationBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationBuilder AppNotificationBuilder::AddArgument(hstring const& key, hstring const& value)
{
THROW_HR_IF_MSG(E_INVALIDARG, key.empty(), "You must provide a key when adding an argument.");
THROW_HR_IF_MSG(E_INVALIDARG, key.empty(), "You must provide a key when adding an argument");

m_arguments.Insert(key, value);
return *this;
Expand All @@ -32,13 +32,13 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation
winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationBuilder AppNotificationBuilder::SetTimeStamp(winrt::Windows::Foundation::DateTime const& value)
{
auto seconds{ winrt::clock::to_time_t(value) };
struct tm buf;
struct tm buf {};
gmtime_s(&buf, &seconds);

std::wstringstream buffer;
buffer << std::put_time(&buf, L"%FT%T");

m_timeStamp = wil::str_printf<std::wstring>(L" displayTimestamp='%wsZ'", buffer.str().c_str());
m_timeStamp = wil::str_printf<std::wstring>(L" displayTimestamp='%lsZ'", buffer.str().c_str());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From @DrusTheAxe in a different PR: Why the trailing 'Z'? Should above be std::put_time(..., L"%FT%T%Z") ?

Copy link
Contributor

Choose a reason for hiding this comment

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

The reason why it's not in std::put_time is because we want the Z to exist as the raw character in the XML to be processed later in AppNotificationBuilder.Show.

return *this;
}

Expand All @@ -56,18 +56,18 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationBuilder AppNotificationBuilder::AddText(hstring const& text)
{
THROW_HR_IF_MSG(E_INVALIDARG, m_textLines.size() >= c_maxTextElements, "Maximum number of text elements added.");
THROW_HR_IF_MSG(E_INVALIDARG, m_textLines.size() >= c_maxTextElements, "Maximum number of text elements added");

m_textLines.push_back(wil::str_printf<std::wstring>(L"<text>%ws</text>", text.c_str()).c_str());
m_textLines.push_back(wil::str_printf<std::wstring>(L"<text>%ls</text>", text.c_str()).c_str());
return *this;
}

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationBuilder AppNotificationBuilder::AddText(hstring const& text, AppNotificationTextProperties const& properties)
{
THROW_HR_IF_MSG(E_INVALIDARG, m_textLines.size() >= c_maxTextElements, "Maximum number of text elements added.");
THROW_HR_IF_MSG(E_INVALIDARG, m_textLines.size() >= c_maxTextElements, "Maximum number of text elements added");

std::wstring props{ properties.as<winrt::Windows::Foundation::IStringable>().ToString() };
m_textLines.push_back(wil::str_printf<std::wstring>(L"%ws%ws</text>", props.c_str(), text.c_str()).c_str());
m_textLines.push_back(wil::str_printf<std::wstring>(L"%ls%ls</text>", props.c_str(), text.c_str()).c_str());

if (properties.IncomingCallAlignment())
{
Expand All @@ -78,29 +78,29 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationBuilder AppNotificationBuilder::SetAttributionText(hstring const& text)
{
m_attributionText = wil::str_printf<std::wstring>(L"<text placement='attribution'>%ws</text>", text.c_str());
m_attributionText = wil::str_printf<std::wstring>(L"<text placement='attribution'>%ls</text>", text.c_str());
return *this;
}

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationBuilder AppNotificationBuilder::SetAttributionText(hstring const& text, hstring const& language)
{
THROW_HR_IF_MSG(E_INVALIDARG, language.empty(), "You must provide a language calling SetAttributionText.");
THROW_HR_IF_MSG(E_INVALIDARG, language.empty(), "You must provide a language calling SetAttributionText");

m_attributionText = wil::str_printf<std::wstring>(L"<text placement='attribution' lang='%ws'>%ws</text>", language.c_str(), text.c_str());
m_attributionText = wil::str_printf<std::wstring>(L"<text placement='attribution' lang='%ls'>%ls</text>", language.c_str(), text.c_str());
return *this;
}

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationBuilder AppNotificationBuilder::SetInlineImage(winrt::Windows::Foundation::Uri const& imageUri)
{
m_inlineImage = wil::str_printf<std::wstring>(L"<image src='%ws'/>", imageUri.ToString().c_str());
m_inlineImage = wil::str_printf<std::wstring>(L"<image src='%ls'/>", imageUri.ToString().c_str());
return *this;
}

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationBuilder AppNotificationBuilder::SetInlineImage(winrt::Windows::Foundation::Uri const& imageUri, AppNotificationImageCrop const& imageCrop)
{
if (imageCrop == AppNotificationImageCrop::Circle)
{
m_inlineImage = wil::str_printf<std::wstring>(L"<image src='%ws' hint-crop='circle'/>", imageUri.ToString().c_str());
m_inlineImage = wil::str_printf<std::wstring>(L"<image src='%ls' hint-crop='circle'/>", imageUri.ToString().c_str());
}
else
{
Expand All @@ -112,25 +112,25 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationBuilder AppNotificationBuilder::SetInlineImage(winrt::Windows::Foundation::Uri const& imageUri, AppNotificationImageCrop const& imageCrop, hstring const& alternateText)
{
THROW_HR_IF_MSG(E_INVALIDARG, alternateText.empty(), "You must provide an alternate text string calling SetInlineImage.");
THROW_HR_IF_MSG(E_INVALIDARG, alternateText.empty(), "You must provide an alternate text string calling SetInlineImage");

std::wstring hintCrop { imageCrop == AppNotificationImageCrop::Circle ? L" hint-crop='circle'" : L"" };
m_inlineImage = wil::str_printf<std::wstring>(L"<image src='%ws' alt='%ws'%ws/>", imageUri.ToString().c_str(), alternateText.c_str(), hintCrop.c_str());
m_inlineImage = wil::str_printf<std::wstring>(L"<image src='%ls' alt='%ls'%ls/>", imageUri.ToString().c_str(), alternateText.c_str(), hintCrop.c_str());

return *this;
}

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationBuilder AppNotificationBuilder::SetAppLogoOverride(winrt::Windows::Foundation::Uri const& imageUri)
{
m_appLogoOverride = wil::str_printf<std::wstring>(L"<image placement='appLogoOverride' src='%ws'/>", imageUri.ToString().c_str());
m_appLogoOverride = wil::str_printf<std::wstring>(L"<image placement='appLogoOverride' src='%ls'/>", imageUri.ToString().c_str());
return *this;
}

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationBuilder AppNotificationBuilder::SetAppLogoOverride(winrt::Windows::Foundation::Uri const& imageUri, AppNotificationImageCrop const& imageCrop)
{
if (imageCrop == AppNotificationImageCrop::Circle)
{
m_appLogoOverride = wil::str_printf<std::wstring>(L"<image placement='appLogoOverride' src='%ws' hint-crop='circle'/>", imageUri.ToString().c_str());
m_appLogoOverride = wil::str_printf<std::wstring>(L"<image placement='appLogoOverride' src='%ls' hint-crop='circle'/>", imageUri.ToString().c_str());
}
else
{
Expand All @@ -142,49 +142,49 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationBuilder AppNotificationBuilder::SetAppLogoOverride(winrt::Windows::Foundation::Uri const& imageUri, AppNotificationImageCrop const& imageCrop, hstring const& alternateText)
{
THROW_HR_IF_MSG(E_INVALIDARG, alternateText.empty(), "You must provide an alternate text string calling SetAppLogoOverride.");
THROW_HR_IF_MSG(E_INVALIDARG, alternateText.empty(), "You must provide an alternate text string calling SetAppLogoOverride");

std::wstring hintCrop{ imageCrop == AppNotificationImageCrop::Circle ? L" hint-crop='circle'" : L"" };
m_appLogoOverride = wil::str_printf<std::wstring>(L"<image placement='appLogoOverride' src='%ws' alt='%ws'%ws/>", imageUri.ToString().c_str(), alternateText.c_str(), hintCrop.c_str());
m_appLogoOverride = wil::str_printf<std::wstring>(L"<image placement='appLogoOverride' src='%ls' alt='%ls'%ls/>", imageUri.ToString().c_str(), alternateText.c_str(), hintCrop.c_str());

return *this;
}

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationBuilder AppNotificationBuilder::SetHeroImage(winrt::Windows::Foundation::Uri const& imageUri)
{
m_heroImage = wil::str_printf<std::wstring>(L"<image placement='hero' src='%ws'/>", imageUri.ToString().c_str());
m_heroImage = wil::str_printf<std::wstring>(L"<image placement='hero' src='%ls'/>", imageUri.ToString().c_str());
return *this;
}

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationBuilder AppNotificationBuilder::SetHeroImage(winrt::Windows::Foundation::Uri const& imageUri, hstring const& alternateText)
{
THROW_HR_IF_MSG(E_INVALIDARG, alternateText.empty(), "You must provide an alternate text string calling SetHeroImage.");
THROW_HR_IF_MSG(E_INVALIDARG, alternateText.empty(), "You must provide an alternate text string calling SetHeroImage");

m_heroImage = wil::str_printf<std::wstring>(L"<image placement='hero' src='%ws' alt='%ws'/>", imageUri.ToString().c_str(), alternateText.c_str());
m_heroImage = wil::str_printf<std::wstring>(L"<image placement='hero' src='%ls' alt='%ls'/>", imageUri.ToString().c_str(), alternateText.c_str());
return *this;
}

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationBuilder AppNotificationBuilder::SetAudioUri(winrt::Windows::Foundation::Uri const& audioUri)
{
m_audio = wil::str_printf<std::wstring>(L"<audio src='%ws'/>", audioUri.ToString().c_str());
m_audio = wil::str_printf<std::wstring>(L"<audio src='%ls'/>", audioUri.ToString().c_str());
return *this;
}

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationBuilder AppNotificationBuilder::SetAudioUri(winrt::Windows::Foundation::Uri const& audioUri, AppNotificationAudioLooping const& loop)
{
m_audio = wil::str_printf<std::wstring>(L"<audio src='%ws' loop='%ws'/>", audioUri.ToString().c_str(), loop == AppNotificationAudioLooping::Loop ? L"true" : L"false");
m_audio = wil::str_printf<std::wstring>(L"<audio src='%ls' loop='%ls'/>", audioUri.ToString().c_str(), loop == AppNotificationAudioLooping::Loop ? L"true" : L"false");
return *this;
}

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationBuilder AppNotificationBuilder::SetAudioEvent(AppNotificationSoundEvent const& soundEvent)
{
m_audio = wil::str_printf<std::wstring>(L"<audio src='%ws'/>", GetWinSoundEventString(soundEvent).c_str());
m_audio = wil::str_printf<std::wstring>(L"<audio src='%ls'/>", GetWinSoundEventString(soundEvent));
return *this;
}

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationBuilder AppNotificationBuilder::SetAudioEvent(AppNotificationSoundEvent const& soundEvent, AppNotificationAudioLooping const& loop)
{
m_audio = wil::str_printf<std::wstring>(L"<audio src='%ws' loop='%ws'/>", GetWinSoundEventString(soundEvent).c_str(), loop == AppNotificationAudioLooping::Loop ? L"true" : L"false");
m_audio = wil::str_printf<std::wstring>(L"<audio src='%ls' loop='%ls'/>", GetWinSoundEventString(soundEvent), loop == AppNotificationAudioLooping::Loop ? L"true" : L"false");
return *this;
}

Expand All @@ -196,7 +196,7 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation

winrt::Microsoft::Windows::AppNotifications::Builder::AppNotificationBuilder AppNotificationBuilder::AddButton(AppNotificationButton const& value)
{
THROW_HR_IF_MSG(E_INVALIDARG, m_buttonList.size() >= c_maxButtonElements, "Maximum number of buttons added.");
THROW_HR_IF_MSG(E_INVALIDARG, m_buttonList.size() >= c_maxButtonElements, "Maximum number of buttons added");

m_buttonList.push_back(value);
return *this;
Expand Down Expand Up @@ -242,21 +242,21 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation
// Add launch arguments if given arguments
if (m_arguments.Size())
{
std::wstring arguments{ };
std::wstring arguments;
for (auto pair : m_arguments)
{
if (!pair.Value().empty())
{
arguments.append(wil::str_printf<std::wstring>(L"%ws=%ws;", pair.Key().c_str(), pair.Value().c_str()));
arguments.append(wil::str_printf<std::wstring>(L"%ls=%ls;", pair.Key().c_str(), pair.Value().c_str()));
}
else
{
arguments.append(wil::str_printf<std::wstring>(L"%ws;", pair.Key().c_str()));
arguments.append(wil::str_printf<std::wstring>(L"%ls;", pair.Key().c_str()));
}
}
arguments.pop_back();

return wil::str_printf<std::wstring>(L" launch='%ws'", arguments.c_str());
return wil::str_printf<std::wstring>(L" launch='%ls'", arguments.c_str());
}
else
{
Expand All @@ -277,7 +277,7 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation

std::wstring AppNotificationBuilder::GetImages()
{
return wil::str_printf<std::wstring>(L"%ws%ws%ws", m_inlineImage.c_str(), m_heroImage.c_str(), m_appLogoOverride.c_str());
return wil::str_printf<std::wstring>(L"%ls%ls%ls", m_inlineImage.c_str(), m_heroImage.c_str(), m_appLogoOverride.c_str());
}

std::wstring AppNotificationBuilder::GetButtons()
Expand All @@ -295,7 +295,7 @@ namespace winrt::Microsoft::Windows::AppNotifications::Builder::implementation
result.append(input.as<winrt::Windows::Foundation::IStringable>().ToString().c_str());
}

return wil::str_printf<std::wstring>(L"<actions>%ws</actions>", result.c_str());
return wil::str_printf<std::wstring>(L"<actions>%ls</actions>", result.c_str());
}
else
{
Expand Down
8 changes: 4 additions & 4 deletions dev/AppNotifications/AppNotificationBuilder/AppNotificationBuilderUtility.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,16 @@
#include "pch.h"
#include "winrt/Microsoft.Windows.AppNotifications.Builder.h"

inline const size_t c_maxAppNotificationPayload{ 5120 };
inline const uint8_t c_maxTextElements{ 3 };
inline const uint8_t c_maxButtonElements{ 5 };
constexpr size_t c_maxAppNotificationPayload{ 5120 };
constexpr uint8_t c_maxTextElements{ 3 };
constexpr uint8_t c_maxButtonElements{ 5 };

namespace AppNotificationBuilder
{
using namespace winrt::Microsoft::Windows::AppNotifications::Builder;
}

inline std::wstring GetWinSoundEventString(AppNotificationBuilder::AppNotificationSoundEvent soundEvent)
inline PCWSTR GetWinSoundEventString(AppNotificationBuilder::AppNotificationSoundEvent soundEvent)
{
switch (soundEvent)
{
Expand Down
Loading