Summary: | Web Inspector: Multiple Selection on Scope Bars by default Conflicts with other behavior on OSX | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brian Weinstein <bweinstein> | ||||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Brian Weinstein <bweinstein> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | aroben, joepeck, rik, timothy | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Attachments: |
|
Description
Brian Weinstein
2009-10-24 17:57:41 PDT
Created attachment 41801 [details]
Fix
At some point, it would be good if these two filter functions were factored into a command function, as they are very similar. I'll file a bug for that and try it in a future patch.
Created attachment 41802 [details]
Fix w/ platform specific modifiers
Created attachment 41803 [details]
Fix + Joe's Comments
Comment on attachment 41803 [details] Fix + Joe's Comments > + Need a short description and bug URL (OOPS!) > + > + No new tests. (OOPS!) Remove. > + // If multiple selection is off, we want to unselect everything else > + // and just select ourselves. > + for (var i = 0; i < this.filterBarElement.childNodes.length; ++i) { > + var child = this.filterBarElement.childNodes[i]; > + if (!child.category) > + continue; > + > + child.removeStyleClass("selected"); > + > + var filterClass = "filter-" + child.category.toLowerCase(); > + this.resourcesGraphsElement.removeStyleClass(filterClass); > + this.resourcesTreeElement.childrenListElement.removeStyleClass(filterClass); > + } You could factor this into a helper nested function that you share with the other part of this function that also deselects all. > if (target.hasStyleClass("selected")) { > + // If selectMultiple is turned on, and we were selected, we just > + // want to unselect ourselves. > target.removeStyleClass("selected"); > - > var filterClass = "filter-" + target.category.toLowerCase(); I prefer to keep the empty line that you deleted, it helps readability. You should also factor out the var filterCalss line so you don't need it twice. Like you did for the ConsoleView. > target.addStyleClass("selected"); > - > var filterClass = "filter-" + target.category.toLowerCase(); Keep that empty line. Created attachment 41811 [details]
Fix + Refactoring
Just wanted one more review on the refactoring for a second set of eyes to make sure I didn't make a silly mistake.
|