Skip to content

Conversation

dcrousso
Copy link
Member

@dcrousso dcrousso commented May 9, 2023

347111d

Web Inspector: REGRESSION(?): Console: find results count has no left padding
https://bugs.webkit.org/show_bug.cgi?id=256531

Reviewed by Patrick Angle.

* Source/WebInspectorUI/UserInterface/Views/FindBanner.css:
(.find-banner):
(.find-banner > input[type="search"]):
(.find-banner > button):
(.find-banner > button.segmented):
(.find-banner > label):
(.find-banner > label:not(:empty)): Added.
(.find-banner > :first-child): Deleted.
(.find-banner > :last-child): Deleted.
(.find-banner.console-find-banner): Deleted.
(body .find-banner.console-find-banner): Deleted.
(.find-banner.console-find-banner > input[type="search"]): Deleted.
(body[dir=ltr] .find-banner.console-find-banner > input[type="search"]): Deleted.
(body[dir=rtl] .find-banner.console-find-banner > input[type="search"]): Deleted.
(.find-banner.console-find-banner > :is(input[type="search"], button)): Deleted.
(.find-banner.console-find-banner > input[type="search"]::-webkit-textfield-decoration-container): Deleted.
(.find-banner.console-find-banner > input[type="search"]:focus, .find-banner.console-find-banner > input[type="search"]:focus ~ button, .find-banner.console-find-banner > input[type="search"]:not(:placeholder-shown), .find-banner.console-find-banner > input[type="search"]:not(:placeholder-shown) ~ button): Deleted.
* Source/WebInspectorUI/UserInterface/Views/LogContentView.css:
(.navigation-bar > .item.find-banner.console): Added.
(:not(.console-drawer) > .navigation-bar > .item.find-banner.console): Added.
(.navigation-bar > .item.find-banner.console > input[type="search"]): Added.
(.navigation-bar > .item.find-banner.console > :is(input[type="search"], button)): Added.
(.navigation-bar > .item.find-banner.console > input[type="search"]::-webkit-textfield-decoration-container): Added.
(.navigation-bar > .item.find-banner.console > input[type="search"]:focus, .navigation-bar > .item.find-banner.console > input[type="search"]:focus ~ button, .navigation-bar > .item.find-banner.console > input[type="search"]:not(:placeholder-shown), .navigation-bar > .item.find-banner.console > input[type="search"]:not(:placeholder-shown) ~ button ): Added.
(.console-find-banner): Deleted.
Avoid using `:first-child`/`:last-child` (and `!important`) as they have very little control over what kinds of situations they apply (e.g. we don't want to add padding to `label:empty`).
Don't add any outside `margin` as that's handled by the containing `WI.NavigationBar` (or adjacent `WI.NavigationItem`) unless we know for sure that it's the first/last `WI.NavigationItem` in the `WI.NavigationBar`.
Drive-by: Move styles to a more relevant location.

* Source/WebInspectorUI/UserInterface/Views/FindBanner.js:
(WI.FindBanner):
* Source/WebInspectorUI/UserInterface/Views/LogContentView.js:
(WI.LogContentView):

Drive-by: Rework optional parameters into `{...} = {}` to match existing pattern elsewhere.
Canonical link: https://commits.webkit.org/264167@main

f061091

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug 🧪 wpe-wk2
✅ 🧪 webkitperl 🧪 ios-wk2 🧪 api-mac ❌ 🛠 gtk
🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ❌ 🧪 gtk-wk2
🧪 api-ios ✅ 🧪 mac-wk2 ❌ 🧪 api-gtk
✅ 🛠 tv 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@dcrousso dcrousso requested a review from patrickangle as a code owner May 9, 2023 18:07
@dcrousso dcrousso self-assigned this May 9, 2023
@dcrousso dcrousso added the Web Inspector Bugs related to the WebKit Web Inspector. label May 9, 2023
Comment on lines 79 to 74
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can use the logical border radius properties here. e.g. border-start-start-radius (https://developer.mozilla.org/en-US/docs/Web/CSS/border-start-start-radius)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually turns out we don't really need these anymore cause of --find-banner-search-box-border-radius-start

@dcrousso dcrousso added the merge-queue Applied to send a pull request to merge-queue label May 17, 2023
… padding

https://bugs.webkit.org/show_bug.cgi?id=256531

Reviewed by Patrick Angle.

* Source/WebInspectorUI/UserInterface/Views/FindBanner.css:
(.find-banner):
(.find-banner > input[type="search"]):
(.find-banner > button):
(.find-banner > button.segmented):
(.find-banner > label):
(.find-banner > label:not(:empty)): Added.
(.find-banner > :first-child): Deleted.
(.find-banner > :last-child): Deleted.
(.find-banner.console-find-banner): Deleted.
(body .find-banner.console-find-banner): Deleted.
(.find-banner.console-find-banner > input[type="search"]): Deleted.
(body[dir=ltr] .find-banner.console-find-banner > input[type="search"]): Deleted.
(body[dir=rtl] .find-banner.console-find-banner > input[type="search"]): Deleted.
(.find-banner.console-find-banner > :is(input[type="search"], button)): Deleted.
(.find-banner.console-find-banner > input[type="search"]::-webkit-textfield-decoration-container): Deleted.
(.find-banner.console-find-banner > input[type="search"]:focus, .find-banner.console-find-banner > input[type="search"]:focus ~ button, .find-banner.console-find-banner > input[type="search"]:not(:placeholder-shown), .find-banner.console-find-banner > input[type="search"]:not(:placeholder-shown) ~ button): Deleted.
* Source/WebInspectorUI/UserInterface/Views/LogContentView.css:
(.navigation-bar > .item.find-banner.console): Added.
(:not(.console-drawer) > .navigation-bar > .item.find-banner.console): Added.
(.navigation-bar > .item.find-banner.console > input[type="search"]): Added.
(.navigation-bar > .item.find-banner.console > :is(input[type="search"], button)): Added.
(.navigation-bar > .item.find-banner.console > input[type="search"]::-webkit-textfield-decoration-container): Added.
(.navigation-bar > .item.find-banner.console > input[type="search"]:focus, .navigation-bar > .item.find-banner.console > input[type="search"]:focus ~ button, .navigation-bar > .item.find-banner.console > input[type="search"]:not(:placeholder-shown), .navigation-bar > .item.find-banner.console > input[type="search"]:not(:placeholder-shown) ~ button ): Added.
(.console-find-banner): Deleted.
Avoid using `:first-child`/`:last-child` (and `!important`) as they have very little control over what kinds of situations they apply (e.g. we don't want to add padding to `label:empty`).
Don't add any outside `margin` as that's handled by the containing `WI.NavigationBar` (or adjacent `WI.NavigationItem`) unless we know for sure that it's the first/last `WI.NavigationItem` in the `WI.NavigationBar`.
Drive-by: Move styles to a more relevant location.

* Source/WebInspectorUI/UserInterface/Views/FindBanner.js:
(WI.FindBanner):
* Source/WebInspectorUI/UserInterface/Views/LogContentView.js:
(WI.LogContentView):

Drive-by: Rework optional parameters into `{...} = {}` to match existing pattern elsewhere.
Canonical link: https://commits.webkit.org/264167@main
@webkit-commit-queue
Copy link
Collaborator

Committed 264167@main (347111d): https://commits.webkit.org/264167@main

Reviewed commits have been landed. Closing PR #13645 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 347111d into WebKit:main May 17, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 17, 2023
@dcrousso dcrousso deleted the eng/256531 branch May 17, 2023 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Web Inspector Bugs related to the WebKit Web Inspector.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants