Bug 138978 - Start using the new :not() and :matches() in the Web Inspector
Summary: Start using the new :not() and :matches() in the Web Inspector
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Benjamin Poulain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-11-21 13:56 PST by Benjamin Poulain
Modified: 2014-11-21 18:06 PST (History)
3 users (show)

See Also:


Attachments
Patch (39.76 KB, patch)
2014-11-21 13:59 PST, Benjamin Poulain
joepeck: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Poulain 2014-11-21 13:56:16 PST
Start using the new :not() and :matches() in the Web Inspector
Comment 1 Benjamin Poulain 2014-11-21 13:59:49 PST
Created attachment 242069 [details]
Patch
Comment 2 Joseph Pecoraro 2014-11-21 17:08:38 PST
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) {
Comment 3 Joseph Pecoraro 2014-11-21 17:10:13 PST
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?
Comment 4 Benjamin Poulain 2014-11-21 18:06:32 PST
Committed r176494: <http://trac.webkit.org/changeset/176494>