Support ScrollView keyboardDismissMode #3665

ddalp · 2019-11-16T00:36:41Z

-Hook up property keyboardDismissMode in ScrollViewManager.
-When manipulation starts and if keyboardDismissMode is on-drag, issue TryHide though SIP event handler.

Some tweak to SIP Event Handler code to be able to share in the RNW's code base, and call GetForUIContext instead of GetForCurrentView on newer build.

Microsoft Reviewers: Open in CodeFlow

licanhua · 2019-11-19T02:57:26Z

vnext/ReactUWP/Views/SIPEventHandler.cpp

+void SIPEventHandler::AttachView(XamlView xamlView, bool fireKeyboardEvents) {
+  m_fireKeyboradEvents = fireKeyboardEvents;
+
+  if (Is19H1OrHigher()) {


Is19H1OrHigher [](start = 6, length = 14)

Suggest you use if (auto uie = try_aswinrt::IUIElement10().
It's possible we don't need to make code change in the future when integrate with winui3. #Resolved

It is there mainly for CoreInputView::GetForUIContext, last heard there is no plan to bring that down level yet.. We can always reevaluate at later time by searching the code where we call the platform check such as Is19H1orHigher..

In reply to: 347710901 [](ancestors = 347710901)

Actually I realized CoreInputView.TryShow/TryHide is not availiable until RS5, so more platform check is needed. And I probably need to fall back to InputPane API on earlier OS.

In reply to: 348209889 [](ancestors = 348209889,347710901)

licanhua · 2019-11-19T03:01:15Z

vnext/ReactUWP/Views/SIPEventHandler.cpp

+  if (m_coreInputView) {
+    auto occlusions = m_coreInputView.GetCoreInputViewOcclusions();
+    m_isShowing = !IsOcclusionsEmpty(occlusions);
+    m_occlusionsChanged_revoker = m_coreInputView.OcclusionsChanged(


OcclusionsChanged [](start = 50, length = 17)

I think this API is introduced since 16299 but we support 15063 #Resolved

licanhua · 2019-11-19T03:03:29Z

vnext/ReactUWP/Views/SIPEventHandler.cpp

-SIPEventHandler::~SIPEventHandler() {
-  m_occlusionsChanged_revoker = {};
+bool SIPEventHandler::TryShow() {
+  if (m_coreInputView == nullptr) {


== nullptr [](start = 21, length = 11)

nit: if (m_coreInputView) #Resolved

licanhua · 2019-11-19T03:03:40Z

vnext/ReactUWP/Views/SIPEventHandler.cpp

+}
+
+bool SIPEventHandler::TryHide() {
+  if (m_coreInputView == nullptr) {


m_coreInputView [](start = 6, length = 15)

nit: if (m_coreInputView) #Resolved

licanhua · 2019-11-19T03:05:42Z

vnext/ReactUWP/Views/SIPEventHandler.h

  std::weak_ptr<IReactInstance> m_wkReactInstance;
  winrt::CoreInputView::OcclusionsChanged_revoker m_occlusionsChanged_revoker;
+  winrt::Rect m_finalRect;
+  winrt::CoreInputView m_coreInputView = nullptr;


winrt::CoreInputView m_coreInputView = nullptr [](start = 2, length = 46)

prefer winrt::CoreInputView m_coreInputView{ nullptr } #Resolved

licanhua · 2019-11-19T03:06:14Z

vnext/ReactUWP/Views/SIPEventHandler.h

  winrt::CoreInputView::OcclusionsChanged_revoker m_occlusionsChanged_revoker;
+  winrt::Rect m_finalRect;
+  winrt::CoreInputView m_coreInputView = nullptr;
+  bool m_isShowing = false;


bool m_isShowing = false [](start = 2, length = 24)

prefer bool m_isShowing{ false } #Resolved

licanhua · 2019-11-20T19:09:44Z

vnext/ReactUWP/Views/ScrollViewManager.cpp

+      m_dismissKeyboardOnDrag = false;
+      if (propertyValue.isString()) {
+        m_dismissKeyboardOnDrag = (propertyValue.getString() == "on-drag");
+        RegisterSIPEventsWhenNeeded();


RegisterSIPEventsWhenNeeded [](start = 8, length = 27)

Do you need to unRegisterSIPEventsWhenNeeded in else? otherwise there is a callback is never be released. #Resolved

It's fine, we tear down event registration in the destructor of keyboardEventHandler, and we release keyboardEventHandler in ~ScrollViewShadowNode.

In reply to: 348693046 [](ancestors = 348693046)

licanhua · 2019-11-20T19:10:25Z

vnext/ReactUWP/Views/ScrollViewManager.cpp

        m_isScrolling = true;
+
+        if (m_dismissKeyboardOnDrag) {
+          if (m_SIPEventHandler) {


m_SIPEventHandler [](start = 14, length = 17)

nit: if (m_dismissKeyboardOnDrag && m_SIPEventHandler) #Resolved

licanhua · 2019-11-20T19:10:48Z

vnext/ReactUWP/Views/ScrollViewManager.cpp

+void ScrollViewShadowNode::RegisterSIPEventsWhenNeeded() {
+  if (m_dismissKeyboardOnDrag) {
+    auto view = GetView();
+    if (winrt::VisualTreeHelper::GetParent(view) != nullptr) {


!= nullptr) [](start = 48, length = 12)

nit: remove !=nullptr #Resolved

licanhua · 2019-11-20T19:19:54Z

vnext/ReactUWP/Views/SIPEventHandler.cpp


-SIPEventHandler::~SIPEventHandler() {
-  m_occlusionsChanged_revoker = {};
+bool SIPEventHandler::TryShow() {


TryShow [](start = 22, length = 7)

I didn't see you can TryShow, why do you need this function? #Resolved

I am expecting to call this function in one of the future ScrollView's API implementation. For now, I will just comment it out.

In reply to: 348697998 [](ancestors = 348697998)

licanhua · 2019-11-20T19:20:42Z

vnext/ReactUWP/Views/SIPEventHandler.cpp

+  return false;
+}
+
+bool SIPEventHandler::TryHide() {


bool [](start = 0, length = 4)

I didn't see you check the result, so just return void instead of bool #Resolved

licanhua · 2019-11-20T19:23:06Z

vnext/ReactUWP/Views/SIPEventHandler.cpp

+      winrt::CoreInputViewOcclusion occlusion = occlusions.GetAt(i);
+      if (occlusion.OcclusionKind() == winrt::CoreInputViewOcclusionKind::Docked) {
+        m_finalRect = winrt::RectHelper::Union(m_finalRect, occlusion.OccludingRect());
+      }


nit: cppwinrt supports for like this
for (auto const& item: occlusions) {
}

or
for (auto item: occlusions) {
}
I don't know if the first one will work or not #Resolved

licanhua

This reverts commit 65f7d80.

* Revert "Support ScrollView keyboardDismissMode (#3665)" This reverts commit 65f7d80. * Change files

Di Da added 2 commits November 15, 2019 16:31

Support keyboardDismissMode on-drag for ScrollView

bfbf63c

Change files

f2d23ef

ddalp requested review from chrisglein, jonthysell, kmelmon, licanhua and marlenecota November 16, 2019 00:36

ddalp requested a review from a team as a code owner November 16, 2019 00:36

ghost added the vnext label Nov 16, 2019

licanhua reviewed Nov 19, 2019

View reviewed changes

Address PR comments and fix two crash

a612f13

licanhua reviewed Nov 20, 2019

View reviewed changes

Address PR comments

2cfce01

licanhua approved these changes Nov 21, 2019

View reviewed changes

ddalp merged commit 65f7d80 into microsoft:master Nov 22, 2019

licanhua added a commit that referenced this pull request Nov 22, 2019

Revert "Support ScrollView keyboardDismissMode (#3665)"

29b555d

This reverts commit 65f7d80.

licanhua mentioned this pull request Nov 22, 2019

Revert "Support ScrollView keyboardDismissMode" #3692

Merged

ghost pushed a commit that referenced this pull request Nov 22, 2019

Revert "Support ScrollView keyboardDismissMode" (#3692)

55ef15e

* Revert "Support ScrollView keyboardDismissMode (#3665)" This reverts commit 65f7d80. * Change files

Support ScrollView keyboardDismissMode #3665

Support ScrollView keyboardDismissMode #3665

Uh oh!

Conversation

ddalp commented Nov 16, 2019 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Microsoft Reviewers: Open in CodeFlow

Uh oh!

licanhua Nov 19, 2019 • edited by ddalp Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ddalp Nov 19, 2019

Choose a reason for hiding this comment

Uh oh!

ddalp Nov 19, 2019

Choose a reason for hiding this comment

Uh oh!

licanhua Nov 19, 2019 • edited by ddalp Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

licanhua Nov 19, 2019 • edited by ddalp Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

licanhua Nov 19, 2019 • edited by ddalp Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

licanhua Nov 19, 2019 • edited by ddalp Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

licanhua Nov 19, 2019 • edited by ddalp Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

licanhua Nov 20, 2019 • edited by ddalp Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ddalp Nov 20, 2019

Choose a reason for hiding this comment

Uh oh!

licanhua Nov 20, 2019 • edited by ddalp Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

licanhua Nov 20, 2019 • edited by ddalp Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

licanhua Nov 20, 2019 • edited by ddalp Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ddalp Nov 20, 2019

Choose a reason for hiding this comment

Uh oh!

licanhua Nov 20, 2019 • edited by ddalp Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

licanhua Nov 20, 2019 • edited by ddalp Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

licanhua left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ddalp commented Nov 16, 2019 •

edited by ghost

Loading

licanhua Nov 19, 2019 •

edited by ddalp

Loading

licanhua Nov 19, 2019 •

edited by ddalp

Loading

licanhua Nov 19, 2019 •

edited by ddalp

Loading

licanhua Nov 19, 2019 •

edited by ddalp

Loading

licanhua Nov 19, 2019 •

edited by ddalp

Loading

licanhua Nov 19, 2019 •

edited by ddalp

Loading

licanhua Nov 20, 2019 •

edited by ddalp

Loading

licanhua Nov 20, 2019 •

edited by ddalp

Loading

licanhua Nov 20, 2019 •

edited by ddalp

Loading

licanhua Nov 20, 2019 •

edited by ddalp

Loading

licanhua Nov 20, 2019 •

edited by ddalp

Loading

licanhua Nov 20, 2019 •

edited by ddalp

Loading