Skip to content

Add UIBlock Interface #1050

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
matthargett merged 11 commits into from Mar 9, 2017
Merged

Add UIBlock Interface #1050

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
94bb735
fix(ReactPage): Fix ApplyArguments to not fail on odd number of argum…
ymusiychuk-lohika Feb 22, 2017
ae2c77a
Syncing with upstream
pre10der89 Mar 1, 2017
8495837
Add UIBlock interface
ryanlntn Mar 3, 2017
9ddb628
Add AddUIBlock methods for UIBlock enqueuing and execution
ryanlntn Mar 3, 2017
4e76093
Support adding UIBlock in WPF too
ryanlntn Mar 5, 2017
38a2ade
Add newline
ryanlntn Mar 7, 2017
2c21d07
Add definition in param doc
ryanlntn Mar 8, 2017
43491c9
Add spaces to param doc
ryanlntn Mar 8, 2017
967e7d6
Merge branch 'master' into uiblock
matthargett Mar 9, 2017
ad5ae4d
Update comments
ryanlntn Mar 9, 2017
04c7308
Merge branch 'uiblock' of github.com:bluejeans/react-native-windows i…
ryanlntn Mar 9, 2017
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
2 changes: 1 addition & 1 deletion ReactWindows/ReactNative.Net46/ReactPage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ public void OnResume(Action onBackPressed)
/// </summary>
public async Task DisposeAsync()
{
RootView?.RemoveHandler(Keyboard.KeyDownEvent, (KeyEventHandler) OnAcceleratorKeyActivated);
RootView?.RemoveHandler(Keyboard.KeyDownEvent, (KeyEventHandler)OnAcceleratorKeyActivated);

if (_reactInstanceManager.IsValueCreated)
{
Expand Down
12 changes: 11 additions & 1 deletion ReactWindows/ReactNative.Net46/UIManager/UIManagerModule.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,17 @@ public int AddMeasuredRootView(SizeMonitoringCanvas rootView)
return tag;
}

#region React Methods
/// <summary>
/// Schedule a block to be executed on the UI thread. Useful if you need to execute
/// view logic after all currently queued view updates have completed.
/// </summary>
/// <param name="block"></param>
public void AddUIBlock(IUIBlock block)

@rozele rozele Mar 9, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Empty comment.

{
_uiImplementation.AddUIBlock(block);
}

#region React Methods

/// <summary>
/// Removes the root view.
Expand Down
1 change: 1 addition & 0 deletions ReactWindows/ReactNative.Shared/ReactNative.Shared.projitems
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@
<Compile Include="$(MSBuildThisFileDirectory)UIManager\BorderedViewParentManager.cs" />
<Compile Include="$(MSBuildThisFileDirectory)UIManager\BorderExtensions.cs" />
<Compile Include="$(MSBuildThisFileDirectory)UIManager\ColorHelpers.cs" />
<Compile Include="$(MSBuildThisFileDirectory)UIManager\IUIBlock.cs" />
<Compile Include="$(MSBuildThisFileDirectory)UIManager\ReactShadowNodeExtensions.cs" />
<Compile Include="$(MSBuildThisFileDirectory)UIManager\ReactShadowNodeVisitor.cs" />
<Compile Include="$(MSBuildThisFileDirectory)UIManager\DependencyObjectExtensions.cs" />
Expand Down
17 changes: 17 additions & 0 deletions ReactWindows/ReactNative.Shared/UIManager/IUIBlock.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
using ReactNative.UIManager;

namespace ReactNative.UIManager
{
/// <summary>
/// Interface that represents a block to execute on the UI thread.
/// Exposes NativeViewHierarchyManager for third party libraries.
/// </summary>
public interface IUIBlock
{
/// <summary>
/// Executes the block.
/// </summary>
/// <param name="nativeViewHierarchyManager">The Native View Hierarchy Manager.</param>
void Execute(NativeViewHierarchyManager nativeViewHierarchyManager);

@rozele rozele Mar 6, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fill in.

@ryanlntn ryanlntn Mar 7, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@rozele Not sure what you mean here. Do you want this method to have a default body or something?

}
}
4 changes: 2 additions & 2 deletions ReactWindows/ReactNative.Shared/UIManager/NativeViewHierarchyManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ public void ShowPopupMenu(int tag, string[] items, ICallback success)
throw new NotImplementedException();
}

private DependencyObject ResolveView(int tag)
public DependencyObject ResolveView(int tag)
{

@rozele rozele Mar 9, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing comments on public classes produce warnings.

var view = default(DependencyObject);
if (!_tagsToViews.TryGetValue(tag, out view))
Expand All @@ -587,7 +587,7 @@ private DependencyObject ResolveView(int tag)
return view;
}

private IViewManager ResolveViewManager(int tag)
public IViewManager ResolveViewManager(int tag)

@rozele rozele Mar 6, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

public [](start = 8, length = 6)

Hmm, I wonder if we should go one step further on Android and create a simple interface that exposes only the methods that we want exposed (e.g., ResolveView and ResolveViewManager). Thoughts @matthargett? #Closed

@matthargett matthargett Mar 6, 2017 •
edited by rozele
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe. I'd need to look at the consumption of the methods being exposed, but it's not jumping out at me in this diff. Also, since its only two methods and they take a POD and return a pure interface, there isn't a coupling problem -- just an ISP problem. #Closed

@ryanlntn ryanlntn Mar 7, 2017 •
edited by rozele
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

While I think that makes sense from a design standpoint I think there is probably more value to having a more similar interfaces between iOS, Android, and Windows as it makes adding Windows support to existing extensions easier. #Closed

{

@rozele rozele Mar 9, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing comments on public classes produce warnings.

var viewManager = default(IViewManager);
if (!_tagsToViewManagers.TryGetValue(tag, out viewManager))
Expand Down
9 changes: 9 additions & 0 deletions ReactWindows/ReactNative.Shared/UIManager/UIImplementation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,15 @@ public void ShowPopupMenu(int reactTag, string[] items, ICallback error, ICallba
_operationsQueue.EnqueueShowPopupMenu(reactTag, items, error, success);
}

/// <summary>
/// Enqueues UIBlock to be executed.
/// </summary>
/// <param name="block"></param>
public void AddUIBlock(IUIBlock block)

@rozele rozele Mar 9, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing comment.

{
_operationsQueue.EnqueueUIBlock(block);
}

/// <summary>
/// Called when the host receives the suspend event.
/// </summary>
Expand Down
9 changes: 9 additions & 0 deletions ReactWindows/ReactNative.Shared/UIManager/UIViewOperationQueue.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,15 @@ public void EnqueueShowPopupMenu(int tag, string[] items, ICallback error, ICall
EnqueueOperation(() => _nativeViewHierarchyManager.ShowPopupMenu(tag, items, success));
}

/// <summary>
/// Enqueues a operation to execute a UIBlock.
/// </summary>
/// <param name="block"></param>
public void EnqueueUIBlock(IUIBlock block)

@rozele rozele Mar 9, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing comment.

{
EnqueueOperation(() => block.Execute(_nativeViewHierarchyManager));
}

/// <summary>
/// Enqueues an operation to create a view.
/// </summary>
Expand Down
12 changes: 11 additions & 1 deletion ReactWindows/ReactNative/UIManager/UIManagerModule.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,17 @@ public int AddMeasuredRootView(SizeMonitoringCanvas rootView)
return tag;
}

#region React Methods
/// <summary>
/// Schedule a block to be executed on the UI thread. Useful if you need to execute
/// view logic after all currently queued view updates have completed.
/// </summary>
/// <param name="block"></param>
public void AddUIBlock(IUIBlock block)

@rozele rozele Mar 9, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Empty comment

{
_uiImplementation.AddUIBlock(block);
}

#region React Methods

/// <summary>
/// Removes the root view.
Expand Down