-
Notifications
You must be signed in to change notification settings - Fork 41
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
Add initial spec of IndexPath class #81
base: master
Are you sure you want to change the base?
Changes from all commits
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,94 @@ | ||
# Background | ||
|
||
To easily track selection in nested collections, we need an object that we can use to uniquely identify an item in nested collections. | ||
This can be done with the IndexPath. | ||
|
||
# Description | ||
|
||
The `IndexPath` class provides indexing of nested collections, for example in a TreeView. | ||
|
||
# Examples | ||
|
||
In the tree below, using the following IndexPath | ||
```c# | ||
|
||
var indexPath = IndexPath.CreateFromIndices(new List<int>{ 1, 0, 3 }); | ||
|
||
``` | ||
would select the item "Kitchen cabinet style", since it is in the second group of first nesting level, first group in the second level of nesting and is the fourth element in the last nesting level. | ||
|
||
![Sample tree selection](./sample-tree-selection.png) | ||
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. Might just be on my end, but the image isn't rendering for me. 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 the webview is not really aware of the relative paths here. Downloading them should work correctly. 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. Weird, it's there, but it doesn't show up. I can vouch that the picture's accurate :) |
||
|
||
### Creating IndexPath objects | ||
New IndexPaths objects can be created using the static `IndexPath.CreateFrom` and `IndexPath.CreateFromIndices` methods: | ||
|
||
```csharp | ||
|
||
// Points to the third element in a flat collection | ||
var simplePath = IndexPath.CreateFrom(2); | ||
|
||
// This is the same IndexPath as "simplePath" | ||
var simplePathIndices = IndexPath.CreateFromIndices(new List<int>(){ 2 }); | ||
|
||
// This will point to the third child of the second element in a nested collection | ||
var groupedPath = IndexPath.CreateFromGroupAndItemIndex(1,2); | ||
|
||
// The following IndexPath is equivalent to "groupedPath" | ||
var groupedPathIndices = IndexPath.CreateFromIndices(new List<int>(){ 1, 2 }); | ||
|
||
``` | ||
|
||
### Comparing two IndexPaths | ||
Comparing two `IndexPath` objects can be done using the `CompareTo` function: | ||
|
||
```c# | ||
var baseIndexPath = IndexPath.CreateFromIndices(new List<int> { 1, 4, 2 }); | ||
|
||
var shorterPath = IndexPath.CreateFromGroupAndItemIndex(2, 0); | ||
|
||
// Since shorterPath takes second element of first nesting, it is "greater" than baseIndexPath | ||
Assert.AreEqual(1, shorterPath.CompareTo(baseIndexPath)); | ||
|
||
|
||
var sameLengthPath = IndexPath.CreateFromIndices(new List<int> { 1, 0, 1 }); | ||
|
||
// Since sameLength path enters before baseIndexPath on the second level, it is smaller than basePath | ||
Assert.AreEqual(-1, sameLengthPath.CompareTo(baseIndexPath)); | ||
|
||
|
||
var baseIndexPathCopy = IndexPath.CreateFromIndices(new List<int> { 1, 4, 2 }); | ||
// Those two are the same, and thus CompareTo returns 0 | ||
Assert.AreEqual(0, baseIndexPathCopy.CompareTo(baseIndexPath)); | ||
``` | ||
|
||
# API Notes | ||
|
||
| Function | Description | | ||
|-|-| | ||
|Int32 GetSize()| Returns the number of nesting levels this IndexPath addresses. | | ||
|Int32 GetAt(Int32 index)| Returns the index for the item in a given nesting level. | | ||
| Int32 CompareTo(IndexPath other)| Compares the IndexPath to a different IndexPath. If the other IndexPath is behind the current IndexPath, the method returns -1, if the other IndexPath is before the current IndexPath it returns 1. In case of equality this method returns 0. | | ||
| static IndexPath CreateFrom(Int32 index) | Creates an IndexPath that only consists of the index provided. | | ||
| static IndexPath CreateFromGroupAndItemIndex(Int32 groupIndex, Int32 itemIndex) | Creates an IndexPath with the given group index and item index for the given group. | | ||
| static IndexPath CreateFromIndices(IVector<Int32> indices) | Creates an IndexPath from the given list of integers. | ||
|
||
# API Details | ||
|
||
```c++ | ||
[WUXC_VERSION_MUXONLY] | ||
[webhosthidden] | ||
runtimeclass IndexPath : Windows.Foundation.IStringable | ||
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 does it need IStringable? 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. It does not really need it, it's just better for debugging and representation. Should we remove it? 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. It is a debugging aid. Perhaps there is a different way to achieve the debug visualization 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 debugging is fine. What does it return? 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. For the IndexPath |
||
{ | ||
Int32 GetSize(); | ||
Int32 GetAt(Int32 index); | ||
Comment on lines
+82
to
+83
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. Should this be an IVectorView? 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. Adding IndexOf doesn't make much sense I think. |
||
Int32 CompareTo(IndexPath other); | ||
|
||
static IndexPath CreateFrom(Int32 index); | ||
static IndexPath CreateFromGroupAndItemIndex(Int32 groupIndex, Int32 itemIndex); | ||
static IndexPath CreateFromIndices(Windows.Foundation.Collections.IVector<Int32> indices); | ||
} | ||
``` | ||
|
||
# Appendix | ||
|
||
Currently we use a `Vector<int>` to store those indices. Since we don't have any operations that would modify the size of an IndexPath, that is how many nesting levels it works with, shouldn't we switch to an array to increase performance? |
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.
Is there a corresponding update to TreeView to accept an IndexPath?
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.
No. TreeView does not use this yet. This is used by SelectionModel, which still needs an API spec. I would make this public along with SelectionModel. We internally use this for NavigationView.
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.
For a feature that cuts across multiple APIs, we likely want a single API spec.
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.
Should we/I add the SelectionModel to this API spec since it is the main API this will interact with (and the only one so far)?