Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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-27-16-11-30-allowRTL.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"type": "prerelease",
"comment": "auto-detect RTL and push into root view",
"packageName": "react-native-windows",
"email": "kmelmon@microsoft.com",
"dependentChangeType": "patch",
"date": "2020-04-27T23:11:30.614Z"
}
3 changes: 1 addition & 2 deletions vnext/Microsoft.ReactNative/ReactHost/ReactInstanceWin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ void ReactInstanceWin::Initialize() noexcept {
strongThis->m_deviceInfo = std::make_shared<react::uwp::DeviceInfo>(legacyInstance);
strongThis->m_appTheme =
std::make_shared<react::uwp::AppTheme>(legacyInstance, strongThis->m_uiMessageThread.LoadWithLock());
strongThis->m_i18nInfo = react::uwp::I18nModule::GetI18nInfo();
react::uwp::I18nHelper().Instance().setInfo(react::uwp::I18nModule::GetI18nInfo());
strongThis->m_appearanceListener = Mso::Make<react::uwp::AppearanceChangeListener>(legacyInstance);
}
})
Expand Down Expand Up @@ -200,7 +200,6 @@ void ReactInstanceWin::Initialize() noexcept {
m_batchingUIThread,
m_uiMessageThread.Load(),
std::move(m_deviceInfo),
std::move(m_i18nInfo),
std::move(m_appState),
std::move(m_appTheme),
std::move(m_appearanceListener),
Expand Down
1 change: 0 additions & 1 deletion vnext/Microsoft.ReactNative/ReactHost/ReactInstanceWin.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ class ReactInstanceWin final : public Mso::ActiveObject<IReactInstanceInternal,
std::shared_ptr<facebook::react::AppState> m_appState;
std::shared_ptr<IRedBoxHandler> m_redboxHandler;
std::shared_ptr<react::uwp::AppTheme> m_appTheme;
std::pair<std::string, bool> m_i18nInfo{};
Mso::CntPtr<react::uwp::AppearanceChangeListener> m_appearanceListener;
std::string m_bundleRootPath;
};
Expand Down
7 changes: 1 addition & 6 deletions vnext/ReactUWP/Base/CoreNativeModules.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ std::vector<facebook::react::NativeModuleDescription> GetCoreModules(
const std::shared_ptr<facebook::react::MessageQueueThread> &messageQueue,
const std::shared_ptr<facebook::react::MessageQueueThread> &uiMessageQueue,
std::shared_ptr<DeviceInfo> &&deviceInfo,
I18nModule::I18nInfo &&i18nInfo,
std::shared_ptr<facebook::react::AppState> &&appstate,
std::shared_ptr<react::uwp::AppTheme> &&appTheme,
Mso::CntPtr<AppearanceChangeListener> &&appearanceListener,
Expand Down Expand Up @@ -119,11 +118,7 @@ std::vector<facebook::react::NativeModuleDescription> GetCoreModules(
messageQueue);

modules.emplace_back(
"I18nManager",
[i18nInfo = std::move(i18nInfo)]() mutable {
return createI18nModule(std::make_unique<I18nModule>(std::move(i18nInfo)));
},
messageQueue);
"I18nManager", []() mutable { return createI18nModule(std::make_unique<I18nModule>()); }, messageQueue);

modules.emplace_back(
AppearanceModule::Name,
Expand Down
1 change: 0 additions & 1 deletion vnext/ReactUWP/Base/CoreNativeModules.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ std::vector<facebook::react::NativeModuleDescription> GetCoreModules(
const std::shared_ptr<facebook::react::MessageQueueThread> &messageQueue,
const std::shared_ptr<facebook::react::MessageQueueThread> &uiMessageQueue,
std::shared_ptr<DeviceInfo> &&deviceInfo,
I18nModule::I18nInfo &&i18nInfo,
std::shared_ptr<facebook::react::AppState> &&appstate,
std::shared_ptr<react::uwp::AppTheme> &&appTheme,
Mso::CntPtr<AppearanceChangeListener> &&appearanceListener,
Expand Down
4 changes: 1 addition & 3 deletions vnext/ReactUWP/Base/UwpReactInstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ void UwpReactInstance::Start(const std::shared_ptr<IReactInstance> &spThis, cons
std::shared_ptr<facebook::react::AppState> appstate = std::make_shared<react::uwp::AppState>(spThis);
std::shared_ptr<react::uwp::AppTheme> appTheme =
std::make_shared<react::uwp::AppTheme>(spThis, m_defaultNativeThread);
std::pair<std::string, bool> i18nInfo = I18nModule::GetI18nInfo();
I18nHelper::Instance().setInfo(I18nModule::GetI18nInfo());
auto appearanceListener = Mso::Make<AppearanceChangeListener>(spThis);

// TODO: Figure out threading. What thread should this really be on?
Expand All @@ -124,7 +124,6 @@ void UwpReactInstance::Start(const std::shared_ptr<IReactInstance> &spThis, cons
spThis,
deviceInfo,
settings,
i18nInfo = std::move(i18nInfo),
appstate = std::move(appstate),
appTheme = std::move(appTheme),
appearanceListener = std::move(appearanceListener)]() mutable {
Expand Down Expand Up @@ -203,7 +202,6 @@ void UwpReactInstance::Start(const std::shared_ptr<IReactInstance> &spThis, cons
m_batchingNativeThread,
m_defaultNativeThread,
std::move(deviceInfo),
std::move(i18nInfo),
std::move(appstate),
std::move(appTheme),
std::move(appearanceListener),
Expand Down
49 changes: 45 additions & 4 deletions vnext/ReactUWP/Modules/I18nModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,55 @@ namespace uwp {
return std::make_pair<std::string, bool>(std::move(locale), std::move(isRTL));
}

I18nModule::I18nModule(std::pair<std::string, bool> &&i18nInfo) : m_i18nInfo(std::move(i18nInfo)) {}
I18nModule::I18nModule() : m_helper(I18nHelper::Instance()) {}

std::string I18nModule::getLocaleIdentifier() {
std::string I18nModule::getLocaleIdentifier() const {
return m_helper.getLocaleIdentifier();
}

bool I18nModule::getIsRTL() const {
return m_helper.getIsRTL();
}

void I18nModule::setAllowRTL(bool allowRTL) {
m_helper.setAllowRTL(allowRTL);
}

void I18nModule::setForceRTL(bool forceRTL) {
m_helper.setForceRTL(forceRTL);
}

/*static*/ I18nHelper &I18nHelper::Instance() {
static I18nHelper theInstance;
return theInstance;
}

I18nHelper::I18nHelper() {}

void I18nHelper::setInfo(I18nModule::I18nInfo &&i18nInfo) {
m_i18nInfo = i18nInfo;
}

std::string I18nHelper::getLocaleIdentifier() const {
return m_i18nInfo.first;
}

bool I18nModule::getIsRTL() {
return m_i18nInfo.second;
bool I18nHelper::getIsRTL() const {
if (m_forceRTL) {
// Used for debugging purposes, forces RTL even in LTR locales
return true;
}

// If the app allows RTL (default is true), then we are in RTL if the locale is RTL
return m_allowRTL && m_i18nInfo.second;
}

void I18nHelper::setAllowRTL(bool allowRTL) {
m_allowRTL = allowRTL;
Copy link
Contributor

@NickGerleman NickGerleman Apr 28, 2020

Choose a reason for hiding this comment

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

Are we okay with this as a global setting instead of per instance? Wondering why we moved to a singleton. #Resolved

Copy link
Contributor Author

@kmelmon kmelmon Apr 28, 2020

Choose a reason for hiding this comment

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

The singleton is there for simplicity and is modeled after how it was done on android. NativeUIManager now depends on functionality inside I18nManager. With the singleton helper we don't need to share the entire I18nModule with the UIManagerModule, which would lead to awkward lifetime issues (the modules are created lazily).

From a high level as to if it's OK to use a singleton, I think so. The information being queried from the OS is per application instance anyway, and it seems not useful for different instances to set allowRTL/forceRTL differently for different instances so simpler seems better to me.

}

void I18nHelper::setForceRTL(bool forceRTL) {
m_forceRTL = forceRTL;
}

} // namespace uwp
Expand Down
31 changes: 27 additions & 4 deletions vnext/ReactUWP/Modules/I18nModule.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,42 @@
namespace react {
namespace uwp {

class I18nHelper;

class I18nModule final : public react::windows::II18nModule {
public:
using I18nInfo = std::pair<std::string, bool>;
static I18nInfo GetI18nInfo(); // Must be called from a UI thread

// II18nModule
I18nModule(I18nInfo &&i18nInfo);
I18nModule();

std::string getLocaleIdentifier() override;
bool getIsRTL() override;
std::string getLocaleIdentifier() const override;
bool getIsRTL() const override;
void setAllowRTL(bool allowRTL) override;
void setForceRTL(bool forceRTL) override;

private:
I18nInfo m_i18nInfo;
I18nHelper &m_helper;
};

class I18nHelper {
public:
static I18nHelper &Instance();
Copy link
Member

Choose a reason for hiding this comment

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

) [](start = 30, length = 1)

Please consider to add noexcept to methods and constructors to keep binary code smaller.


I18nHelper();

void setInfo(I18nModule::I18nInfo &&i18nInfo);
std::string getLocaleIdentifier() const;
bool getIsRTL() const;
void setAllowRTL(bool allowRTL);
void setForceRTL(bool forceRTL);

private:
I18nModule::I18nInfo m_i18nInfo;
bool m_allowRTL{true};
bool m_forceRTL{false};
};

} // namespace uwp
} // namespace react
9 changes: 8 additions & 1 deletion vnext/ReactUWP/Modules/NativeUIManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include "pch.h"

#include "I18nModule.h"
#include "NativeUIManager.h"

#include <ReactRootView.h>
Expand Down Expand Up @@ -185,6 +186,10 @@ void NativeUIManager::AddRootView(
XamlView view = xamlRootView->GetXamlView();
m_tagsToXamlReactControl.emplace(shadowNode.m_tag, xamlRootView->GetXamlReactControl());

// Push the appropriate FlowDirection into the root view.
view.as<xaml::FrameworkElement>().FlowDirection(
I18nHelper::Instance().getIsRTL() ? xaml::FlowDirection::RightToLeft : xaml::FlowDirection::LeftToRight);

m_tagsToYogaNodes.emplace(shadowNode.m_tag, make_yoga_node());

auto element = view.as<xaml::FrameworkElement>();
Expand Down Expand Up @@ -851,7 +856,9 @@ void NativeUIManager::DoLayout() {
float actualWidth = static_cast<float>(rootElement.ActualWidth());
float actualHeight = static_cast<float>(rootElement.ActualHeight());

// TODO: Real direction (VSO 1697992: RTL Layout)
// We must always run layout in LTR mode, which might seem unintuitive.
// We will flip the root of the tree into RTL by forcing the root XAML node's FlowDirection to RightToLeft
// which will inherit down the XAML tree, allowing all native controls to pick it up.
YGNodeCalculateLayout(rootNode, actualWidth, actualHeight, YGDirectionLTR);

for (auto &tagToYogaNode : m_tagsToYogaNodes) {
Expand Down
10 changes: 8 additions & 2 deletions vnext/ReactWindowsCore/Modules/I18nModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
// Licensed under the MIT License.

#include "I18nModule.h"
#include <cxxreact/JsArgumentHelpers.h>

using namespace facebook::xplat;

namespace react {
namespace windows {
Expand All @@ -18,12 +21,15 @@ std::map<std::string, folly::dynamic> I18nModule::getConstants() {
}

std::vector<facebook::xplat::module::CxxModule::Method> I18nModule::getMethods() {
return {};
return {
Method("allowRTL", [this](folly::dynamic args) { this->m_module->setAllowRTL(jsArgAsBool(args, 0)); }),
Method("forceRTL", [this](folly::dynamic args) { this->m_module->setForceRTL(jsArgAsBool(args, 0)); }),
};
}

std::unique_ptr<facebook::xplat::module::CxxModule> createI18nModule(std::unique_ptr<II18nModule> module) {
return std::make_unique<I18nModule>(std::move(module));
}

} // namespace windows
} // namespace react
} // namespace react
8 changes: 5 additions & 3 deletions vnext/include/ReactWindowsCore/II18nModule.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@ namespace windows {

struct II18nModule {
virtual ~II18nModule(){};
virtual std::string getLocaleIdentifier() = 0;
virtual bool getIsRTL() = 0;
virtual std::string getLocaleIdentifier() const = 0;
virtual bool getIsRTL() const = 0;
virtual void setAllowRTL(bool allowRTL) = 0;
virtual void setForceRTL(bool forceRTL) = 0;
};

std::unique_ptr<facebook::xplat::module::CxxModule> createI18nModule(std::unique_ptr<II18nModule> module);
} // namespace windows
} // namespace react
} // namespace react