Skip to content

Implement String ViewManager Command IDs #4667

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

Merged
NickGerleman merged 3 commits into from
Apr 22, 2020
Merged

Implement String ViewManager Command IDs #4667

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
25d8c22
Implement String ViewManager Command IDs
NickGerleman Apr 22, 2020
7341d26
Merge branch 'master' of https://github.com/microsoft/react-native-wi…
NickGerleman Apr 22, 2020
b105db4
Change files
NickGerleman Apr 22, 2020
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
8 changes: 8 additions & 0 deletions change/react-native-windows-2020-04-22-15-02-43-stringviewmanagertemp.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"type": "prerelease",
"comment": "Implement String ViewManager Command IDs",
"packageName": "react-native-windows",
"email": "ngerlem@microsoft.com",
"dependentChangeType": "patch",
"date": "2020-04-22T22:02:43.043Z"
}
1 change: 0 additions & 1 deletion packages/microsoft-reactnative-sampleapps/windows/SampleAppCPP/SampleAppCpp.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
<PropertyGroup>
<ReactNativeWindowsDir Condition="'$(ReactNativeWindowsDir)' == ''">$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), 'node_modules\react-native-windows\package.json'))\node_modules\react-native-windows\</ReactNativeWindowsDir>
</PropertyGroup>
<Import Project="$(ReactNativeWindowsDir)PropertySheets\React.Cpp.props" />
Copy link
Contributor Author

@NickGerleman NickGerleman Apr 21, 2020

Choose a reason for hiding this comment

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

We shouldn't include React.cpp.props in projects external to the Microsoft.ReactNative boundary. We don't include it in the template project, and including it here actually led to it dropiing a ReactCommon folder on my C drive.

<ItemGroup Label="ProjectConfigurations">
<ProjectConfiguration Include="Debug|ARM">
<Configuration>Debug</Configuration>
Expand Down
10 changes: 5 additions & 5 deletions ...osoft-reactnative-sampleapps/windows/SampleLibraryCPP/CustomUserControlViewManagerCPP.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,18 +105,18 @@ void CustomUserControlViewManagerCpp::UpdateProperties(
}

// IViewManagerWithCommands
IMapView<hstring, int64_t> CustomUserControlViewManagerCpp::Commands() noexcept {
auto commands = winrt::single_threaded_map<hstring, int64_t>();
commands.Insert(L"CustomCommand", 0);
IVectorView<hstring> CustomUserControlViewManagerCpp::Commands() noexcept {
auto commands = winrt::single_threaded_vector<hstring>();
commands.Append(L"CustomCommand");
return commands.GetView();
}

void CustomUserControlViewManagerCpp::DispatchCommand(
Copy link
Member

@asklar asklar Apr 22, 2020

Choose a reason for hiding this comment

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

since we are going to support both the int and string variants for the time being, should this be an overload rather than changing it to string so we can test both cases?

Copy link
Contributor Author

@NickGerleman NickGerleman Apr 22, 2020

Choose a reason for hiding this comment

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

I touched on that a little in the PR description. Internal CPP code needs to support both, but the C++ view manager exposing IDs to implementers is a departure from the C# and ObjC view managers at least.

This has some motivation to move the direction of only strings, but the bigger concern I had was around API confusion. I.e. you look at an interface and see a command id version and a string version. It's super opaque to know which to implement when.

FrameworkElement const &view,
int64_t commandId,
winrt::hstring const &commandId,
IJSValueReader const &commandArgsReader) noexcept {
if (auto control = view.try_as<winrt::SampleLibraryCpp::CustomUserControlCpp>()) {
if (commandId == 0) {
if (commandId == L"CustomCommand") {
std::string arg = std::to_string(winrt::unbox_value<int64_t>(view.Tag()));
arg.append(", \"");
arg.append(winrt::to_string(commandArgsReader.GetString()));
Expand Down
4 changes: 2 additions & 2 deletions ...crosoft-reactnative-sampleapps/windows/SampleLibraryCPP/CustomUserControlViewManagerCPP.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ struct CustomUserControlViewManagerCpp
winrt::Microsoft::ReactNative::IJSValueReader const &propertyMapReader) noexcept;

// IViewManagerWithCommands
winrt::Windows::Foundation::Collections::IMapView<winrt::hstring, int64_t> Commands() noexcept;
winrt::Windows::Foundation::Collections::IVectorView<winrt::hstring> Commands() noexcept;

void DispatchCommand(
winrt::Windows::UI::Xaml::FrameworkElement const &view,
int64_t commandId,
winrt::hstring const &commandId,
winrt::Microsoft::ReactNative::IJSValueReader const &commandArgsReader) noexcept;

// IViewManagerWithExportedEventTypeConstants
Expand Down
1 change: 0 additions & 1 deletion packages/microsoft-reactnative-sampleapps/windows/SampleLibraryCPP/SampleLibraryCPP.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
<WindowsTargetPlatformMinVersion>10.0.16299.0</WindowsTargetPlatformMinVersion>
</PropertyGroup>
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.Default.props" />
<Import Project="$(ReactNativeWindowsDir)PropertySheets\React.Cpp.props" />
<ItemGroup Label="ProjectConfigurations">
<ProjectConfiguration Include="Debug|ARM">
<Configuration>Debug</Configuration>
Expand Down
2 changes: 1 addition & 1 deletion vnext/Desktop.DLL/react-native-win32.x64.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ EXPORTS
?createIUIManager@react@facebook@@YA?AV?$shared_ptr@VIUIManager@react@facebook@@@std@@$$QEAV?$vector@V?$unique_ptr@VIViewManager@react@facebook@@U?$default_delete@VIViewManager@react@facebook@@@std@@@std@@V?$allocator@V?$unique_ptr@VIViewManager@react@facebook@@U?$default_delete@VIViewManager@react@facebook@@@std@@@std@@@2@@4@PEAUINativeUIManager@12@@Z
?createUIManagerModule@react@facebook@@YA?AV?$unique_ptr@VCxxModule@module@xplat@facebook@@U?$default_delete@VCxxModule@module@xplat@facebook@@@std@@@std@@V?$shared_ptr@VIUIManager@react@facebook@@@4@@Z
?destroy@dynamic@folly@@AEAAXXZ
?dispatchCommand@ShadowNode@react@facebook@@UEAAX_JAEBUdynamic@folly@@@Z
?dispatchCommand@ShadowNode@react@facebook@@UEAAXAEBV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@AEBUdynamic@folly@@@Z
?getModuleRegistry@Instance@react@facebook@@QEAAAEAVModuleRegistry@23@XZ
?get_ptr@dynamic@folly@@QEGBAPEBU12@V?$Range@PEBD@2@@Z
?get_ptrImpl@dynamic@folly@@AEGBAPEBU12@AEBU12@@Z
Expand Down
2 changes: 1 addition & 1 deletion vnext/Desktop.DLL/react-native-win32.x86.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ EXPORTS
?createIUIManager@react@facebook@@YG?AV?$shared_ptr@VIUIManager@react@facebook@@@std@@$$QAV?$vector@V?$unique_ptr@VIViewManager@react@facebook@@U?$default_delete@VIViewManager@react@facebook@@@std@@@std@@V?$allocator@V?$unique_ptr@VIViewManager@react@facebook@@U?$default_delete@VIViewManager@react@facebook@@@std@@@std@@@2@@4@PAUINativeUIManager@12@@Z
?createUIManagerModule@react@facebook@@YG?AV?$unique_ptr@VCxxModule@module@xplat@facebook@@U?$default_delete@VCxxModule@module@xplat@facebook@@@std@@@std@@V?$shared_ptr@VIUIManager@react@facebook@@@4@@Z
?destroy@dynamic@folly@@AAEXXZ
?dispatchCommand@ShadowNode@react@facebook@@UAEX_JABUdynamic@folly@@@Z
?dispatchCommand@ShadowNode@react@facebook@@UAEXABV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@ABUdynamic@folly@@@Z
?getModuleRegistry@Instance@react@facebook@@QAEAAVModuleRegistry@23@XZ
?get_ptr@dynamic@folly@@QGBEPBU12@V?$Range@PBD@2@@Z
?get_ptrImpl@dynamic@folly@@AGBEPBU12@ABU12@@Z
Expand Down
5 changes: 4 additions & 1 deletion vnext/Desktop.UnitTests/EmptyUIManagerModule.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ class EmptyUIManager {
std::function<void()> /*final Callback*/ callback);
void setJSResponder(int64_t reactTag, bool blockNativeResponder);
void clearJSResponder();
void dispatchViewManagerCommand(int64_t reactTag, int64_t commandId, folly::dynamic /*ReadableMap*/ commandArgs);
void dispatchViewManagerCommand(
int64_t reactTag,
const std::string &commandId,
folly::dynamic /*ReadableMap*/ commandArgs);
void showPopupMenu(
int64_t reactTag,
folly::dynamic /*ReadableMap*/ items,
Expand Down
25 changes: 9 additions & 16 deletions vnext/Microsoft.ReactNative.SharedManaged/AttributedViewManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using Windows.UI.Xaml;
Expand Down Expand Up @@ -192,27 +193,21 @@ private static bool IsEnum(Type t)

#region Commands

public virtual IReadOnlyDictionary<string, long> Commands
public virtual IReadOnlyList<string> Commands
{
get
{
if (null == _commands)
{
var commands = new Dictionary<string, long>();
_commands = ViewManagerCommands.Keys.ToList();

foreach (var kvp in ViewManagerCommands)
{
commands.Add(kvp.Value.CommandName, kvp.Value.CommandId);
}

_commands = commands;
}
return _commands;
}
}
private IReadOnlyDictionary<string, long> _commands;
private IReadOnlyList<string> _commands;

public virtual void DispatchCommand(FrameworkElement view, long commandId, IJSValueReader commandArgsReader)
public virtual void DispatchCommand(FrameworkElement view, string commandId, IJSValueReader commandArgsReader)
{
if (view is TFrameworkElement viewAsT)
{
Expand All @@ -227,13 +222,13 @@ public virtual void DispatchCommand(FrameworkElement view, long commandId, IJSVa
}
}

internal Dictionary<long, ViewManagerCommand<TFrameworkElement>> ViewManagerCommands
internal Dictionary<string, ViewManagerCommand<TFrameworkElement>> ViewManagerCommands
{
get
{
if (null == _viewManagerCommands)
{
var viewManagerCommands = new Dictionary<long, ViewManagerCommand<TFrameworkElement>>();
var viewManagerCommands = new Dictionary<string, ViewManagerCommand<TFrameworkElement>>();

foreach (var methodInfo in GetType().GetTypeInfo().DeclaredMethods)
{
Expand All @@ -243,10 +238,9 @@ internal Dictionary<long, ViewManagerCommand<TFrameworkElement>> ViewManagerComm
var command = new ViewManagerCommand<TFrameworkElement>
{
CommandName = commandAttribute.CommandName ?? methodInfo.Name,
CommandId = viewManagerCommands.Count,
CommandMethod = MakeReaderMethod(methodInfo)
};
viewManagerCommands.Add(command.CommandId, command);
viewManagerCommands.Add(command.CommandName, command);
}
}

Expand All @@ -255,12 +249,11 @@ internal Dictionary<long, ViewManagerCommand<TFrameworkElement>> ViewManagerComm
return _viewManagerCommands;
}
}
private Dictionary<long, ViewManagerCommand<TFrameworkElement>> _viewManagerCommands;
private Dictionary<string, ViewManagerCommand<TFrameworkElement>> _viewManagerCommands;

internal struct ViewManagerCommand<U> where U : TFrameworkElement
{
public string CommandName;
public long CommandId;
public Action<U, IJSValueReader> CommandMethod;
}

Expand Down
20 changes: 11 additions & 9 deletions vnext/Microsoft.ReactNative/ABIViewManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,29 +115,31 @@ void ABIViewManager::UpdateProperties(react::uwp::ShadowNodeBase *nodeToUpdate,
}

folly::dynamic ABIViewManager::GetCommands() const {
folly::dynamic innerParent = Super::GetCommands();
Copy link
Contributor Author

@NickGerleman NickGerleman Apr 21, 2020

Choose a reason for hiding this comment

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

Our superclass always returned nothing, so removing it doesn't do anything in practice, but I don't think this should have been here to begin with.

I.e. we don't dispatch to the superclass, so even if it did has commands to report, we could never actually run them.

Copy link
Member

@asklar asklar Apr 22, 2020

Choose a reason for hiding this comment

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

shouldn't we fix the "dispatching to the superclass isn't working" problem then? Can you file an issue if you can't fix it in this PR?

Copy link
Contributor Author

@NickGerleman NickGerleman Apr 22, 2020

Choose a reason for hiding this comment

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

I think it's fine to expect the Base View Manager won't handle anything in this case. Right now the only thing it does is assert(false) as a method to catch unhandled commands.

folly::dynamic commandMap = folly::dynamic::object();

// Why are we providing commands with the same key and value? React Native 0.61 internally introduced string command
// IDs which can be dispatched directly without querying the ViewManager for commands. Integer command IDs are
// internally deprecated, but querying for command ID is still the documented path. Returning constants as their
// string lets us internally only support the string path.
if (m_viewManagerWithCommands) {
auto outerChild = m_viewManagerWithCommands.Commands();
for (const auto &pair : outerChild) {
std::string key = to_string(pair.Key());
folly::dynamic value{pair.Value()};
innerParent.insert(key, value);
for (const auto &commandName : m_viewManagerWithCommands.Commands()) {
auto commandAsStr = to_string(commandName);
commandMap[commandAsStr] = commandAsStr;
Copy link
Contributor Author

@NickGerleman NickGerleman Apr 22, 2020

Choose a reason for hiding this comment

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

On the Flow side, View Manager commands are untyped. Was unable to confirm whether other platforms have moved to fully using strings over the bridge yet though.

}
}

return innerParent;
return commandMap;
}

void ABIViewManager::DispatchCommand(
winrt::Windows::UI::Xaml::DependencyObject viewToUpdate,
int64_t commandId,
const std::string &commandId,
const folly::dynamic &commandArgs) {
if (m_viewManagerWithCommands) {
auto view = viewToUpdate.as<winrt::FrameworkElement>();

IJSValueReader argReader = winrt::make<DynamicReader>(commandArgs);
m_viewManagerWithCommands.DispatchCommand(view, commandId, argReader);
m_viewManagerWithCommands.DispatchCommand(view, to_hstring(commandId), argReader);
}
}

Expand Down
2 changes: 1 addition & 1 deletion vnext/Microsoft.ReactNative/ABIViewManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class ABIViewManager : public react::uwp::FrameworkElementViewManager {

void DispatchCommand(
winrt::Windows::UI::Xaml::DependencyObject viewToUpdate,
int64_t commandId,
const std::string &commandId,
const folly::dynamic &commandArgs) override;

folly::dynamic GetExportedCustomBubblingEventTypeConstants() const override;
Expand Down
6 changes: 3 additions & 3 deletions vnext/Microsoft.ReactNative/IViewManager.idl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,17 @@ namespace Microsoft.ReactNative
[webhosthidden]
interface IViewManagerWithNativeProperties
{
Windows.Foundation.Collections.IMapView<String, ViewManagerPropertyType> NativeProps { get; };
IMapView<String, ViewManagerPropertyType> NativeProps { get; };

void UpdateProperties(Windows.UI.Xaml.FrameworkElement view, IJSValueReader propertyMapReader);
}

[webhosthidden]
interface IViewManagerWithCommands
{
Windows.Foundation.Collections.IMapView<String, Int64> Commands { get; };
IVectorView<String> Commands { get; };

void DispatchCommand(Windows.UI.Xaml.FrameworkElement view, Int64 commandId, IJSValueReader commandArgsReader);
void DispatchCommand(Windows.UI.Xaml.FrameworkElement view, String commandId, IJSValueReader commandArgsReader);
}

[webhosthidden]
Expand Down
50 changes: 22 additions & 28 deletions vnext/ReactUWP/Views/ScrollViewManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,18 @@
namespace react {
namespace uwp {

enum class ScrollViewCommands {
ScrollTo = 1,
ScrollToEnd,
};
namespace ScrollViewCommands {
constexpr const char *ScrollTo = "scrollTo";
constexpr const char *ScrollToEnd = "scrollToEnd";
}; // namespace ScrollViewCommands

class ScrollViewShadowNode : public ShadowNodeBase {
using Super = ShadowNodeBase;

public:
ScrollViewShadowNode();
~ScrollViewShadowNode();
void dispatchCommand(int64_t commandId, const folly::dynamic &commandArgs) override;
void dispatchCommand(const std::string &commandId, const folly::dynamic &commandArgs) override;
void createView() override;
void updateProperties(const folly::dynamic &&props) override;

Expand Down Expand Up @@ -65,28 +65,23 @@ ScrollViewShadowNode::~ScrollViewShadowNode() {
m_SIPEventHandler.reset();
}

void ScrollViewShadowNode::dispatchCommand(int64_t commandId, const folly::dynamic &commandArgs) {
void ScrollViewShadowNode::dispatchCommand(const std::string &commandId, const folly::dynamic &commandArgs) {
const auto scrollViewer = GetView().as<winrt::ScrollViewer>();
if (scrollViewer == nullptr)
return;

switch (commandId) {
case static_cast<int64_t>(ScrollViewCommands::ScrollTo): {
double x = commandArgs[0].asDouble();
double y = commandArgs[1].asDouble();
bool animated = commandArgs[2].asBool();
scrollViewer.ChangeView(x, y, nullptr, !animated /*disableAnimation*/);
break;
}
case static_cast<int64_t>(ScrollViewCommands::ScrollToEnd): {
bool animated = commandArgs[0].asBool();
bool horiz = scrollViewer.HorizontalScrollMode() == winrt::ScrollMode::Auto;
if (horiz)
scrollViewer.ChangeView(scrollViewer.ScrollableWidth(), nullptr, nullptr, !animated /*disableAnimation*/);
else
scrollViewer.ChangeView(nullptr, scrollViewer.ScrollableHeight(), nullptr, !animated /*disableAnimation*/);
break;
}
if (commandId == ScrollViewCommands::ScrollTo) {
double x = commandArgs[0].asDouble();
double y = commandArgs[1].asDouble();
bool animated = commandArgs[2].asBool();
scrollViewer.ChangeView(x, y, nullptr, !animated /*disableAnimation*/);
} else if (commandId == ScrollViewCommands::ScrollToEnd) {
bool animated = commandArgs[0].asBool();
bool horiz = scrollViewer.HorizontalScrollMode() == winrt::ScrollMode::Auto;
if (horiz)
scrollViewer.ChangeView(scrollViewer.ScrollableWidth(), nullptr, nullptr, !animated /*disableAnimation*/);
else
scrollViewer.ChangeView(nullptr, scrollViewer.ScrollableHeight(), nullptr, !animated /*disableAnimation*/);
}
}

Expand Down Expand Up @@ -413,11 +408,10 @@ const char *ScrollViewManager::GetName() const {
}

folly::dynamic ScrollViewManager::GetCommands() const {
auto commands = Super::GetCommands();
commands.update(folly::dynamic::object(
"scrollTo", static_cast<std::underlying_type_t<ScrollViewCommands>>(ScrollViewCommands::ScrollTo))(
"scrollToEnd", static_cast<std::underlying_type_t<ScrollViewCommands>>(ScrollViewCommands::ScrollToEnd)));
return commands;
// Upstream JS will dispatch the string directly instead of ever actually calling this, but providing a real
// implementation is simple enough in case anything changes.
return folly::dynamic::object(ScrollViewCommands::ScrollTo, ScrollViewCommands::ScrollTo)(
ScrollViewCommands::ScrollToEnd, ScrollViewCommands::ScrollToEnd);
}

folly::dynamic ScrollViewManager::GetNativeProps() const {
Expand Down
2 changes: 1 addition & 1 deletion vnext/ReactUWP/Views/ShadowNodeBase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ bool ShadowNodeBase::NeedsForceLayout() {
return false;
}

void ShadowNodeBase::dispatchCommand(int64_t commandId, const folly::dynamic &commandArgs) {
void ShadowNodeBase::dispatchCommand(const std::string &commandId, const folly::dynamic &commandArgs) {
GetViewManager()->DispatchCommand(GetView(), commandId, commandArgs);
}

Expand Down
12 changes: 12 additions & 0 deletions vnext/ReactUWP/Views/SwitchViewManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,5 +160,17 @@ bool SwitchViewManager::UpdateProperty(
return true;
}

void SwitchViewManager::DispatchCommand(
XamlView viewToUpdate,
const std::string &commandId,
const folly::dynamic &commandArgs) {
if (commandId == "setValue") {
auto value = commandArgs[0].asBool();
viewToUpdate.as<winrt::ToggleSwitch>().IsEnabled(value);
Copy link
Contributor Author

@NickGerleman NickGerleman Apr 21, 2020

Choose a reason for hiding this comment

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

This command is kind of weird. It's iOS only upstream, and in their impl doesn't touch native props either. From what I can tell it's to fix an iOS quirk, but implementing it doesn't seem to cause us any issues, and let's us fully share iOS JS code.

} else {
Super::DispatchCommand(viewToUpdate, commandId, commandArgs);
}
}

} // namespace uwp
} // namespace react
1 change: 1 addition & 0 deletions vnext/ReactUWP/Views/SwitchViewManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class SwitchViewManager : public ControlViewManager {
const char *GetName() const override;
folly::dynamic GetNativeProps() const override;
facebook::react::ShadowNode *createShadow() const override;
void DispatchCommand(XamlView viewToUpdate, const std::string &commandId, const folly::dynamic &commandArgs) override;

protected:
bool UpdateProperty(
Expand Down
2 changes: 1 addition & 1 deletion vnext/ReactUWP/Views/ViewManagerBase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ void ViewManagerBase::TransferProperties(XamlView /*oldView*/, XamlView /*newVie

void ViewManagerBase::DispatchCommand(
XamlView /*viewToUpdate*/,
Copy link
Member

@vmoroz vmoroz Apr 22, 2020

Choose a reason for hiding this comment

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

XamlView [](start = 4, length = 8)

Since it is a new method, could you pass the parameter correctly by const& ?

int64_t /*commandId*/,
const std::string & /*commandId*/,
const folly::dynamic & /*commandArgs*/) {
assert(false); // View did not handle its command
}
Expand Down
3 changes: 2 additions & 1 deletion vnext/ReactWindowsCore/IUIManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ class IUIManager {
folly::dynamic &addChildTags,
folly::dynamic &addAtIndices,
folly::dynamic &removeFrom) = 0;
virtual void dispatchViewManagerCommand(int64_t reactTag, int64_t commandId, folly::dynamic &&commandArgs) = 0;
virtual void
dispatchViewManagerCommand(int64_t reactTag, const std::string &commandId, folly::dynamic &&commandArgs) = 0;
virtual void replaceExistingNonRootView(int64_t oldTag, int64_t newTag) = 0;
virtual void measure(int64_t reactTag, facebook::xplat::module::CxxModule::Callback callback) = 0;
virtual void measureInWindow(int64_t reactTag, facebook::xplat::module::CxxModule::Callback callback) = 0;
Expand Down
Loading