Add a spec for pane navigation #8375
Conversation
zadjii-msft
added
Issue-Docs
|
Thanks so much for typing all of this up! I'm a fan of proposal E :) |
|
Update: Consensus from team sync was proposal D |
|
I believe we also want to change |
|
(this one's ready for review) |
## Summary of the Pull Request I think we all agree that the current spec template doesn't always work. I thought this layout might be better for the kinds of settings discussions we have (more and more frequently now). This is largely for discussion with the team - if there are other things we want added, changed, or if we just want to merge this in with the primary spec template, I'm all ears. ## References * An example of using this spec: #8375
zadjii-msft
assigned DHowett, carlos-zamora, cinnamon-msft and leonMSFT and unassigned leonMSFT
| Additionally, we concurred that the new "direction" value should be `prev`, not | ||
| `last`, for consistency's sake. |
There was a problem hiding this comment.
nit: you could just update Proposal D to use prev instead of last. But meh.
| These movements would necessarily need to be in-order traversals, since there's | ||
| no way of doing multiple MRU steps. |
There was a problem hiding this comment.
Is in-order traversal...
- order the panes were opened, or
- order the panes were interacted with?
I'd like a bit more discussion on the second one. I still don't understand the issue with the following approach:
- maintain a linked list of weak references to Panes
- prev/next navigates the list
- if a pane was removed, skip the node
- if a pane receives focus (except from prev/next action)...
- clear all "next" panes from the list
- replace with a weak ref to the newly focused pane
In my mind, that's how Ctrl+Z/Y works in text editors and how Back/Next work in web browsers.
There was a problem hiding this comment.
"in-order" is always "the order defined by the pane IDs" and "MRU order" is always the "order in which they were last interacted with".
I don't know if your comment was from before we discussed this in the meeting or not. TL;DR: Without another transient UI to manage the navigation of the MRU stack, it's not possible to know the difference between focus moving because of MRU pane navigation, or the user interacting with it. We'd have to hook all the possible interaction events for the control, and then manually handle them as "commit this navigation", and it's just a mess
ghost
added
the
Needs-Author-Feedback
ghost
added
the
No-Recent-Activity
|
This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment. |
zadjii-msft
removed
the
No-Recent-Activity
ghost
removed
the
Needs-Author-Feedback
|
@DHowett @carlos-zamora Should we just merge this with the two? we already shipped the code... |
zadjii-msft
changed the title
## Summary of the Pull Request This is a spec for "pane navigation", as we've already got a bit of an implementation in microsoft#8183. We've also had a heated discussion in Teams, and I wanted to capture a bit of that in a more formal doc. I suppose that "informal Teams chat" didn't work out in the end 😆. Also, this is @PankajBhojwani's feature so I'm gonna let him drive. I mostly wrote this to test out a new spec template. After discussion, we landed on proposal D, with a minor change of `last` to `prev`. This is how it was in microsoft#8183 before I started meddling 😝 ## PR Checklist * [x] spec for microsoft#2871 * [x] I work here ## Detailed Description of the Pull Request / Additional comments This is not my best spec ever - again, mostly just trying to spawn discussion, and prototype the new spec template.
Summary of the Pull Request
This is a spec for "pane navigation", as we've already got a bit of an implementation in #8183. We've also had a heated discussion in Teams, and I wanted to capture a bit of that in a more formal doc. I suppose that "informal Teams chat" didn't work out in the end 😆.
Also, this is @PankajBhojwani's feature so I'm gonna let him drive. I mostly wrote this to test out a new spec template.
After discussion, we landed on proposal D, with a minor change of
lasttoprev. This is how it was in #8183 before I started meddling 😝PR Checklist
Detailed Description of the Pull Request / Additional comments
This is not my best spec ever - again, mostly just trying to spawn discussion, and prototype the new spec template.