Bug 205033 - Web Inspector: REGRESSION(r253167): Elements: class list toggle doesn't stay sticky to the bottom of the sidebar
Summary: Web Inspector: REGRESSION(r253167): Elements: class list toggle doesn't stay ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
: 205031 (view as bug list)
Depends on: 204690
Blocks:
  Show dependency treegraph
 
Reported: 2019-12-09 16:05 PST by Devin Rousso
Modified: 2019-12-10 10:39 PST (History)
6 users (show)

See Also:


Attachments
[Image] Screenshot of issue (62.51 KB, image/png)
2019-12-09 16:05 PST, Devin Rousso
no flags Details
Patch (2.33 KB, patch)
2019-12-09 16:09 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (1.29 KB, patch)
2019-12-09 17:05 PST, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2019-12-09 16:05:16 PST
Created attachment 385200 [details]
[Image] Screenshot of issue

.
Comment 1 Devin Rousso 2019-12-09 16:09:38 PST
Created attachment 385202 [details]
Patch
Comment 2 Devin Rousso 2019-12-09 16:10:34 PST
*** Bug 205031 has been marked as a duplicate of this bug. ***
Comment 3 Nikita Vasilyev 2019-12-09 16:48:27 PST
Comment on attachment 385202 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=385202&action=review

> Source/WebInspectorUI/ChangeLog:15
> +        * UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.css:
> +        (.spreadsheet-css-declaration:nth-last-child(n of .spreadsheet-css-declaration)): Added.
> +        Drive-by: remove the bottom border from the last section since there's a top border on both
> +                  the class list toggle area and the filter bar.

That's not what it does. It removes the border after every section.

I don't think we should remove this subtle 0.5px border for the last section. The background of the panel is different from the section background — I don't think it looks good without the border.
Comment 4 Devin Rousso 2019-12-09 16:56:50 PST
Comment on attachment 385202 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=385202&action=review

>> Source/WebInspectorUI/ChangeLog:15
>> +                  the class list toggle area and the filter bar.
> 
> That's not what it does. It removes the border after every section.
> 
> I don't think we should remove this subtle 0.5px border for the last section. The background of the panel is different from the section background — I don't think it looks good without the border.

I suppose.  I was trying to avoid a double-border when the content of the sidebar is scrolled all the way to the bottom.  I'll think of other alternatives.

> Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.css:37
> +.spreadsheet-css-declaration:nth-last-child(n of .spreadsheet-css-declaration) {

Ah, whoops.  I meant to use `1` instead of `n`.
Comment 5 Nikita Vasilyev 2019-12-09 16:57:50 PST
Comment on attachment 385202 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=385202&action=review

>> Source/WebInspectorUI/ChangeLog:15
>> +                  the class list toggle area and the filter bar.
> 
> That's not what it does. It removes the border after every section.
> 
> I don't think we should remove this subtle 0.5px border for the last section. The background of the panel is different from the section background — I don't think it looks good without the border.

(Also, I had to read MDN to undestand what this selector is supposed to do. It seems like this could be `.spreadsheet-css-declaration:last-of-type`.)
Comment 6 Devin Rousso 2019-12-09 17:05:50 PST
Created attachment 385213 [details]
Patch
Comment 7 Nikita Vasilyev 2019-12-09 17:06:01 PST
Comment on attachment 385202 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=385202&action=review

>>>> Source/WebInspectorUI/ChangeLog:15
>>>> +                  the class list toggle area and the filter bar.
>>> 
>>> That's not what it does. It removes the border after every section.
>>> 
>>> I don't think we should remove this subtle 0.5px border for the last section. The background of the panel is different from the section background — I don't think it looks good without the border.
>> 
>> I suppose.  I was trying to avoid a double-border when the content of the sidebar is scrolled all the way to the bottom.  I'll think of other alternatives.
> 
> (Also, I had to read MDN to undestand what this selector is supposed to do. It seems like this could be `.spreadsheet-css-declaration:last-of-type`.)

I agree that we shouldn't have a double-border when scrolled all the way to the bottom.

It didn't have double borders when we used absolute positioning:

.sidebar > .panel.details.css-style > .content.has-filter-bar {
    bottom: calc(var(--navigation-bar-height) - 1px);
}
Comment 8 Nikita Vasilyev 2019-12-09 17:06:55 PST
Comment on attachment 385213 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=385213&action=review

> Source/WebInspectorUI/UserInterface/Views/GeneralStyleDetailsSidebarPanel.css:27
> +    flex-grow: 1;

This certanly fixes the regression.
Comment 9 BJ Burg 2019-12-10 09:52:45 PST
Comment on attachment 385213 [details]
Patch

r=me
Comment 10 WebKit Commit Bot 2019-12-10 10:38:48 PST
Comment on attachment 385213 [details]
Patch

Clearing flags on attachment: 385213

Committed r253330: <https://trac.webkit.org/changeset/253330>
Comment 11 WebKit Commit Bot 2019-12-10 10:38:49 PST
All reviewed patches have been landed.  Closing bug.
Comment 12 Radar WebKit Bug Importer 2019-12-10 10:39:52 PST
<rdar://problem/57799143>