-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix two crashes from NativeAnimatedExamples #3400
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.
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
Changes from all commits
63529c8
62049fd
1d2404c
e1d1e32
893ad25
50abb31
08a650a
4660f7d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| { | ||
| "type": "prerelease", | ||
| "comment": "Fix two issues: 1) you cannot animated 2 subchannels of the same property with different animations. to fix this we animated yet another property set for translation and scale owned by the props nodes and use one animation to animate all of the subchannels for the uiElement. 2) Reference parameter names which started with a multi digit number are unsupported so i added an n to the start of each name, which was previously just the node's tag.", | ||
| "packageName": "react-native-windows", | ||
| "email": "stpete@microsoft.com", | ||
| "commit": "62049fdbd667fc71ae09b09f074446d8593d826d", | ||
| "date": "2019-10-11T20:21:32.900Z" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| { | ||
| "type": "patch", | ||
| "comment": "Fix two issues: 1) you cannot animated 2 subchannels of the same property with different animations. to fix this we animated yet another property set for translation and scale owned by the props nodes and use one animation to animate all of the subchannels for the uiElement. 2) Reference parameter names which started with a multi digit number are unsupported so i added an n to the start of each name, which was previously just the node's tag.", | ||
| "packageName": "react-native-windows-extended", | ||
| "email": "stpete@microsoft.com", | ||
| "commit": "62049fdbd667fc71ae09b09f074446d8593d826d", | ||
| "date": "2019-10-11T20:21:24.946Z" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| { | ||
| "type": "patch", | ||
| "comment": "Fix two issues: 1) you cannot animated 2 subchannels of the same property with different animations. to fix this we animated yet another property set for translation and scale owned by the props nodes and use one animation to animate all of the subchannels for the uiElement. 2) Reference parameter names which started with a multi digit number are unsupported so i added an n to the start of each name, which was previously just the node's tag.", | ||
| "packageName": "rnpm-plugin-windows", | ||
| "email": "stpete@microsoft.com", | ||
| "commit": "62049fdbd667fc71ae09b09f074446d8593d826d", | ||
| "date": "2019-10-11T20:20:58.182Z" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| { | ||
| "type": "none", | ||
| "comment": "Fix two issues: 1) you cannot animated 2 subchannels of the same property with different animations. to fix this we animated yet another property set for translation and scale owned by the props nodes and use one animation to animate all of the subchannels for the uiElement. 2) Reference parameter names which started with a multi digit number are unsupported so i added an n to the start of each name, which was previously just the node's tag.", | ||
| "packageName": "rnpm-plugin-wpf", | ||
| "email": "stpete@microsoft.com", | ||
| "commit": "62049fdbd667fc71ae09b09f074446d8593d826d", | ||
| "date": "2019-10-11T20:21:05.364Z" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| #include "pch.h" | ||
|
|
||
| #include <Views/ShadowNodeBase.h> | ||
| #include "SliderViewManager.h" | ||
|
|
||
| #include <Utils/ValueUtils.h> | ||
|
|
||
| #include <IReactInstance.h> | ||
|
|
||
| #include <winrt/Windows.UI.Xaml.Controls.Primitives.h> | ||
|
|
||
| namespace winrt { | ||
| using ToggleButton = Windows::UI::Xaml::Controls::Primitives::ToggleButton; | ||
| } | ||
|
|
||
| namespace react { | ||
| namespace uwp { | ||
|
|
||
| class SliderShadowNode : public ShadowNodeBase { | ||
| using Super = ShadowNodeBase; | ||
|
|
||
| public: | ||
| SliderShadowNode() = default; | ||
| void createView() override; | ||
| void updateProperties(const folly::dynamic &&props) override; | ||
| }; | ||
|
|
||
| void SliderShadowNode::createView() { | ||
| Super::createView(); | ||
| } | ||
|
|
||
| void SliderShadowNode::updateProperties(const folly::dynamic &&props) { | ||
| m_updating = true; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
why do you need m_updating flag? #Pending
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All the shadow nodes do this, i couldn't tell you why In reply to: 335127109 [](ancestors = 335127109) |
||
| Super::updateProperties(std::move(props)); | ||
| m_updating = false; | ||
| } | ||
|
|
||
| SliderViewManager::SliderViewManager( | ||
| const std::shared_ptr<IReactInstance> &reactInstance) | ||
| : Super(reactInstance) {} | ||
|
|
||
| const char *SliderViewManager::GetName() const { | ||
| return "RCTSlider"; | ||
| } | ||
|
|
||
| folly::dynamic SliderViewManager::GetNativeProps() const { | ||
| auto props = Super::GetNativeProps(); | ||
|
|
||
| props.update( | ||
| folly::dynamic::object("value", "integer")("disabled", "boolean")); | ||
|
|
||
| return props; | ||
| } | ||
|
|
||
| facebook::react::ShadowNode *SliderViewManager::createShadow() const { | ||
| return new SliderShadowNode(); | ||
| } | ||
|
|
||
| XamlView SliderViewManager::CreateViewCore(int64_t tag) { | ||
| auto slider = winrt::Slider(); | ||
| return slider; | ||
| } | ||
|
|
||
| void SliderViewManager::UpdateProperties( | ||
| ShadowNodeBase *nodeToUpdate, | ||
| const folly::dynamic &reactDiffMap) { | ||
| auto slider = nodeToUpdate->GetView().as<winrt::Slider>(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I guess you means try_as. otherwise as would crash if it's null #Resolved |
||
| if (slider == nullptr) | ||
| return; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need to do this check. #Pending
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we do, as and try_as return null if the QI fails In reply to: 335128424 [](ancestors = 335128424) |
||
|
|
||
| for (const auto &pair : reactDiffMap.items()) { | ||
| const std::string &propertyName = pair.first.getString(); | ||
| const folly::dynamic &propertyValue = pair.second; | ||
|
|
||
| if (propertyName == "disabled") { | ||
| if (propertyValue.isBool()) | ||
| slider.IsEnabled(!propertyValue.asBool()); | ||
| else if (pair.second.isNull()) | ||
| slider.ClearValue(winrt::Control::IsEnabledProperty()); | ||
| } else if (propertyName == "value") { | ||
| if (propertyValue.isNumber()) | ||
| slider.Value(propertyValue.asDouble()); | ||
| else if (pair.second.isNull()) | ||
| slider.Value(0); | ||
| } | ||
| } | ||
|
|
||
| Super::UpdateProperties(nodeToUpdate, reactDiffMap); | ||
| } | ||
|
|
||
| } // namespace uwp | ||
| } // namespace react | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| #pragma once | ||
|
|
||
| #include <Views/ControlViewManager.h> | ||
|
|
||
| namespace react { | ||
| namespace uwp { | ||
|
|
||
| class SliderViewManager : public ControlViewManager { | ||
| using Super = ControlViewManager; | ||
|
|
||
| public: | ||
| SliderViewManager(const std::shared_ptr<IReactInstance> &reactInstance); | ||
|
|
||
| const char *GetName() const override; | ||
| folly::dynamic GetNativeProps() const override; | ||
|
|
||
| facebook::react::ShadowNode *createShadow() const override; | ||
|
|
||
| void UpdateProperties( | ||
| ShadowNodeBase *nodeToUpdate, | ||
| const folly::dynamic &reactDiffMap) override; | ||
|
|
||
| protected: | ||
| XamlView CreateViewCore(int64_t tag) override; | ||
|
|
||
| friend class SliderShadowNode; | ||
| }; | ||
|
|
||
| } // namespace uwp | ||
| } // namespace react |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#include <Views/SliderViewManager.h> ? #Pending
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not how any of the other view managers include their header
In reply to: 335125441 [](ancestors = 335125441)