Summary: | Web Inspector: Group selectors to highlight matched selector in the Styles pane of Elements Panel | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexander Pavlov (apavlov) <apavlov> | ||||||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Alexander Pavlov (apavlov) <apavlov> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | apavlov, bweinstein, doekman, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, vsevik, yurys | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | All | ||||||||||||||
OS: | All | ||||||||||||||
Attachments: |
|
Description
Alexander Pavlov (apavlov)
2012-09-13 03:00:34 PDT
Created attachment 164080 [details]
Patch
Created attachment 164081 [details]
[IMAGE] Screenshot of matching group selectors with the patch applied
I would dim non-matching selectors instead. Comment on attachment 164080 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164080&action=review > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1154 > + var selectors = selectorText.split(",").map(trim); var selectors = selectorText.split(",").map(String.prototype.trim); > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1177 > + if (matches && node === paneNode) s/paneNode/this._parentPane.node/ > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1178 > + selectors[selectorIndex] = "<span class=\"selector-matches\">" + selectors[selectorIndex].escapeHTML() + "</span>"; Let's create DOM element and setContent instead. > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1179 > + if (isLast) { selectorIndex === selectors.length - 1 > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1180 > + self._selectorElement.innerHTML = selectors.join(", "); We should use appendChild Comment on attachment 164080 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164080&action=review >> Source/WebCore/inspector/front-end/StylesSidebarPane.js:1180 >> + self._selectorElement.innerHTML = selectors.join(", "); > > We should use appendChild +1. Let's agree that any assignment of innerHTML to non-empy string is an automatic r- in the inspector codebase. (In reply to comment #6) > (From update of attachment 164080 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=164080&action=review > > >> Source/WebCore/inspector/front-end/StylesSidebarPane.js:1180 > >> + self._selectorElement.innerHTML = selectors.join(", "); > > > > We should use appendChild > > +1. Let's agree that any assignment of innerHTML to non-empy string is an automatic r- in the inspector codebase. I don't think there is a valid reason to use innerHTML for empty strings. element.textContent = "" FTW Created attachment 164143 [details]
Patch
Created attachment 164156 [details]
[IMAGE] Screenshot - take 2
Non-matching selectors are dimmed. When a section is hovered, all non-matching selectors turn black to improve readability. Inherited element styles' selectors are matched against their respective elements.
UI lgtm. Why is html rendered as matching in the inherited from body section? (In reply to comment #10) > UI lgtm. Why is html rendered as matching in the inherited from body section? It's because the mouse pointer is hovering over the section. In that case we un-dim the dimmed selectors to aid users with poor monitors :) and eyesight. Seva said that we should not regress the UI readability in general (that is, it should be possible to view the UI that is not harder to read than it was previously.) I would only make it solid upon selector editing. I think dimmed selector is still very distinguishable. (In reply to comment #12) > I would only make it solid upon selector editing. I think dimmed selector is still very distinguishable. I trust your eyesight! :) Uploading an updated patch shortly. Created attachment 164359 [details]
Patch
Committed r128746: <http://trac.webkit.org/changeset/128746> I don't see why we need to make 2 additional requests per matched selectors call. We should change the protocol to return ready-to-use data instead. (In reply to comment #16) > I don't see why we need to make 2 additional requests per matched selectors call. We should change the protocol to return ready-to-use data instead. Are you suggesting to send indices of matched selectors in groups up-front? When sending rules, we can send selectors in the model form (array of entities withe the flag responsible for those matching). *** Bug 31211 has been marked as a duplicate of this bug. *** |