Community
Participate
Working Groups
I was chatting with Susan yesterday about moving diff sections under staged/unstaged sections. The initial thought was that diffs under staged/unstaged would be still sections but rendered differently. Right now we use explorer to display the list of diffs under staged/unstaged sections. The same that we use on git-log page or in File Navigator. So if we just replace the explorer with diff sections , we would lose all this fancy stuff we have from explorer, like selection support. So maybe we should just use an enhanced explorer there. it means the top-level elements would be diffs and their children would be compare views. We would have twisties for free from explorer and one thing missing would be progress tracking. But I think we already have something for progress indication in explorers, for instance Choose a Folder dialog on Sites page.
Talked to Simon about this. This should work. - he is having widget lifecycle issues with the compare widget. It needs the parent div to be in the DOM, whereas the explorer/treetable will not insert rows in the DOM until everything is built. So we'll need some kind of lifecycle events for the explorer/treetable to tell a renderer when the row is inserted in the dom. I can do this, but we should probably spend some time talking about the lifecycle of compare and why this is not possible (is it the editor or something else?) - we'll need to talk to Libing about the selection model, though. With this approach, it means a "row" in the explorer is not participating in selection, because the parent above it is really the only thing that can take selection. It's like a "parent only" selection model. - another oddity is that we'll want to cursor (up/down) past the child compare widget, yet we might want to traverse (tab) to it???? I'm not sure what the model should be here.
Created attachment 215554 [details] Diffs moved I released some changes. Remaining issues are: - get rid of the line between the change name and the compare view (1). This seems to be related to the discussion about selection model that treats some rows in a special way. - action icons should be aligned to the right (2) - remove checkboxes in favor of the new selection model (3) Moreover the editor is initialized with a timeout to workaround the problem with "the parent div in the DOM". This is just for now before we decide how to solve the root cause of the problem.
I'm leaving the bug open for now. I hope to close it once we address issues mentioned in comment 1 and comment 2.
(In reply to comment #2) > Created attachment 215554 [details] > Diffs moved > > I released some changes. Remaining issues are: > - get rid of the line between the change name and the compare view (1). This > seems to be related to the discussion about selection model that treats some > rows in a special way. You should be able to get rid of these lines completely by using decorateAlternatingLines: false in your renderer. > - action icons should be aligned to the right (2) A bigger question I think is whether we want to keep the inline actions at all. Following the rules we posited in bug 367458 comment 7, I think we may end up with these icons going away, and the user would simply select bulk to stage/unstage. > - remove checkboxes in favor of the new selection model (3) For gut checking the visuals you can use checkbox: false in your renderer, but you won't get any other way to do selection until you hook in Libing's model. > Moreover the editor is initialized with a timeout to workaround the problem > with "the parent div in the DOM". This is just for now before we decide how to > solve the root cause of the problem. We'll probably open a separate bug on this after the fact. I feel like we know what to do for git-status2 *except* for the "I have have rows that aren't selections" issue...We will have to figure out if the child compare widget is truly an explorer child node, or if instead this is just a special kind of row that opens wider...
*** Bug 378686 has been marked as a duplicate of this bug. ***
the "selection model for free" behavior is now implemented. Libing is still working out some bugs, but I did try git-status 2 and was able to see that mouse selection is working (without checkboxes). So I think you can safely now: - set your renderer options to include: {decorateAlternatingLines: false, checkbox: false} - get rid of the item level command contributions and just use them in the section header. You want to recheck your commands' visibleWhen and callback implementations, because you'll now get an array of selections in the callback instead of a single item. I found in nav that some commands were not dealing with empty arrays (no selection) or with arrays at all.... At that point I believe that the scoped selections should work as you expect, besides bugs we are seeing with the selection model in general. So it seems like our main remaining issue is to figure out a way for an explorer to say "this item is not in the row selection." Maybe this is already possible, Libing can tell us...that way you could prevent traversal into the compare widgets.
Created attachment 215621 [details] decorateAlternatingLines issue I removed checkboxes, but decorateAlternatingLines does not exactly do what I want. The screenshot shows the idea. I would expect that if decorateAlternatingLines:false is used the line will be in place 2 instead of place 1.
(In reply to comment #4) > > Moreover the editor is initialized with a timeout to workaround the problem > > with "the parent div in the DOM". This is just for now before we decide how to > > solve the root cause of the problem. > We'll probably open a separate bug on this after the fact. Raised Bug 379510. I'm not sure if we should remove item actions completely. I wanted to use item actions for compare widget actions. Moreover having stage/unstage actions next to each change could be useful, especially when you have many changes, comapre widgets opened and you are at the end of the list. Without item actions, you would need to select the change, scroll up and then click stage. Anyway, the question for now is where to put compare widget actions?
talked to Szymon about this, it seems like the compare widget actions might look nice there. Then the question is whether they appear/disappear with the widget. Our feeling is that the "compare" link should always be there, but the view controls (next diff,etc.) would appear only with the widget.
I raised Bug 379672 for alternating lines, so compare widget actions is the only remaining issue in this bug now. I tried to render compare widget actions in the parent actions div. It works, but: - commands are visible all the time, even if the compare widget is collapsed - I use commandSpanId and set it to the parent actions div id. When I traverse the page using tab and enter the unstaged(staged) list and click tab again, the selection is changed to the first compare widget action instead of leaving the section. See the screenshot.
Created attachment 215699 [details] Wrong traversing
Marking this bug fixed. I raised Bug 379811 for issues with compare widget in explorer.
