Bug 112404 - Web Inspector: DataGrid should reveal and select next/previous DataGridNode upon deletion of selected node
Summary: Web Inspector: DataGrid should reveal and select next/previous DataGridNode u...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-14 22:12 PDT by Sankeerth V S
Modified: 2013-03-18 05:28 PDT (History)
10 users (show)

See Also:


Attachments
Patch (3.84 KB, patch)
2013-03-15 03:33 PDT, Sankeerth V S
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sankeerth V S 2013-03-14 22:12:51 PDT
While deleting the entries of the data grid by clicking on 'Delete' status bar button, deletion of selected node should trigger selection of
next possible (backward/forward) DataGridNode.
Comment 1 Sankeerth V S 2013-03-15 03:33:50 PDT
Created attachment 193272 [details]
Patch
Comment 2 WebKit Review Bot 2013-03-15 06:37:28 PDT
Comment on attachment 193272 [details]
Patch

Clearing flags on attachment: 193272

Committed r145901: <http://trac.webkit.org/changeset/145901>
Comment 3 WebKit Review Bot 2013-03-15 06:37:32 PDT
All reviewed patches have been landed.  Closing bug.
Comment 4 Vivek Galatage 2013-03-17 22:35:37 PDT
In my opinion, node selection change should happen inherently within the DataGrid whenever a node is removed. Making an explicit call to changeNodeAfterDeletion() from the call site (e.g. _deleteButtonClicked) shouldn't be necessary. 

Also when the DataGrid is used in multiple views with the inline deleting option, and if the call site doesn't invoke changeNodeAfterDeletion() explicitly, the DataGrid behavior would be inconsistent with respect to other views and this would hamper the User experience.

I would recommend that this method changeNodeAfterDeletion() be moved in the DataGridNode and make it a private API with more meaningful naming such as ( _nodeSelectionChanged() ) and should be invoked from WebInspector.DataGridNode.removeChild() implicitly.

@vsevik, I would be glad to have your views on the above comment.
Comment 5 Vsevolod Vlasov 2013-03-18 00:33:53 PDT
(In reply to comment #4)
> In my opinion, node selection change should happen inherently within the DataGrid whenever a node is removed. Making an explicit call to changeNodeAfterDeletion() from the call site (e.g. _deleteButtonClicked) shouldn't be necessary. 
> 
> Also when the DataGrid is used in multiple views with the inline deleting option, and if the call site doesn't invoke changeNodeAfterDeletion() explicitly, the DataGrid behavior would be inconsistent with respect to other views and this would hamper the User experience.
> 
> I would recommend that this method changeNodeAfterDeletion() be moved in the DataGridNode and make it a private API with more meaningful naming such as ( _nodeSelectionChanged() ) and should be invoked from WebInspector.DataGridNode.removeChild() implicitly.
> 
> @vsevik, I would be glad to have your views on the above comment.

Sounds resanobale, would you be willing to file a new bug a implement this?
Comment 6 Sankeerth V S 2013-03-18 05:26:19 PDT
(In reply to comment #4)
> In my opinion, node selection change should happen inherently within the DataGrid whenever a node is removed. Making an explicit call to changeNodeAfterDeletion() from the call site (e.g. _deleteButtonClicked) shouldn't be necessary. 
> 
> Also when the DataGrid is used in multiple views with the inline deleting option, and if the call site doesn't invoke changeNodeAfterDeletion() explicitly, the DataGrid behavior would be inconsistent with respect to other views and this would hamper the User experience.
> 
> I would recommend that this method changeNodeAfterDeletion() be moved in the DataGridNode and make it a private API with more meaningful naming such as ( _nodeSelectionChanged() ) and should be invoked from WebInspector.DataGridNode.removeChild() implicitly.
> 
> @vsevik, I would be glad to have your views on the above comment.

Thank you for your valuable suggestions. What you mentioned is is right. I hadn't thought about it before. I shall incorporate the changes you have mentioned.
Comment 7 Sankeerth V S 2013-03-18 05:28:48 PDT
> Sounds reasonable, would you be willing to file a new bug a implement this?

Sure. I shall open a new bug and implement the above mentioned changes.
Thank you.