| Summary: | Start using the new :not() and :matches() in the Web Inspector | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Benjamin Poulain <benjamin> | ||||
| Component: | New Bugs | Assignee: | Benjamin Poulain <benjamin> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | joepeck, jonowells, timothy | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Benjamin Poulain
2014-11-21 13:56:16 PST
Created attachment 242069 [details]
Patch
Comment on attachment 242069 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=242069&action=review r=me > Source/WebInspectorUI/UserInterface/Views/ControlToolbarItem.css:43 > +.toolbar:matches(.icon-and-label-horizontal, .toolbar.icon-only) .item.control { Oops, an extra ".toolbar" was left in here. Should be: .toolbar:matches(.icon-and-label-horizontal, .icon-only) .item.control > Source/WebInspectorUI/UserInterface/Views/DebuggerDashboardView.css:133 > +.toolbar.collapsed .dashboard.debugger > :not(.message, .navigation-bar ) { Nit: You can remove the extra trailing space in the :not. > Source/WebInspectorUI/UserInterface/Views/DefaultDashboardView.css:28 > -body.web .toolbar.collapsed .dashboard.default > .time, > -body.web .toolbar.collapsed .dashboard.default > .resourcesSize, > -body.web .toolbar.collapsed .dashboard.default > .logs { > +body.web .toolbar.collapsed .dashboard.default > :matches(.time, .resourcesSize, .logs) { > display: none; > } These are awesome cases. > Source/WebInspectorUI/UserInterface/Views/DefaultDashboardView.css:213 > -.toolbar.label-only .dashboard.default > .item, > -.toolbar.small-size.icon-only .dashboard.default > .item, > -.toolbar.small-size.icon-and-label-vertical .dashboard.default > .item, > -.toolbar.small-size.icon-and-label-horizontal .dashboard.default > .item { > +.toolbar:matches(.label-only, .small-size:matches(.icon-only, .icon-and-label-vertical, .icon-and-label-horizontal)) .dashboard.default > .item { These are less awesome because it becomes confusing to read. But I'm fine with it on the few cases. > Source/WebInspectorUI/UserInterface/Views/FindBanner.css:52 > +:matches(.find-banner, .supports-find-banner).no-find-banner-transition { I think in our code we will want to be stylistically consistent and always have :matches as a suffix. So could you change this to: .no-find-banner-transition:matches(.find-banner, .supports-find-banner) { Also it is a bit misleading to enable this feature by default in a patch named "use :not and :matches in the inspector". Did you intend to split that up into two patches? Committed r176494: <http://trac.webkit.org/changeset/176494> |