Bug 227707 - Web Inspector: Elements Tab Details Sidebar navigation items sometime wrap to a second line
Summary: Web Inspector: Elements Tab Details Sidebar navigation items sometime wrap to...
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: Patrick Angle
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-07-06 09:03 PDT by Patrick Angle
Modified: 2023-06-14 04:53 PDT (History)
4 users (show)

See Also:


Attachments
Screen Recording of Issue (33.26 MB, video/quicktime)
2021-07-06 09:03 PDT, Patrick Angle
no flags Details
Patch v1.0 (4.81 KB, patch)
2021-07-06 11:22 PDT, Patrick Angle
no flags Details | Formatted Diff | Diff
Screen Recording of Patch v1.0 (11.38 MB, video/quicktime)
2021-07-06 11:30 PDT, Patrick Angle
no flags Details
Patch v1.1 - Review notes (4.78 KB, patch)
2021-07-06 12:12 PDT, Patrick Angle
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Angle 2021-07-06 09:03:27 PDT
Created attachment 432938 [details]
Screen Recording of Issue

At very narrow widths, the navigation items for the details sidebar can wrap, resulting in a strange-looking appearance. It appears adding the `Layout` item revealed this existing bug.
Comment 1 Radar WebKit Bug Importer 2021-07-06 09:03:48 PDT
<rdar://problem/80213546>
Comment 2 Patrick Angle 2021-07-06 11:22:51 PDT
Created attachment 432948 [details]
Patch v1.0
Comment 3 Patrick Angle 2021-07-06 11:30:08 PDT
Created attachment 432951 [details]
Screen Recording of Patch v1.0
Comment 4 Devin Rousso 2021-07-06 11:39:52 PDT
Comment on attachment 432948 [details]
Patch v1.0

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

r=me, nice fix :)

> Source/WebInspectorUI/ChangeLog:19
> +         When accumulating item widths, use the ceiling of the width to make up for slight imprecision in cumulative
> +        sizing in order to get sizing just a bit larger than the item, instead of possibly just a bit smaller.

NIT: this is basically a repeat of the above, so I'd either remove the above or remove this

> Source/WebInspectorUI/UserInterface/Views/NavigationBar.js:226
> +            return visibleNavigationItems.reduce((total, item) => total + Math.ceil(item.width), 0);

There's another `navigationItem.width` on :255.  Should we `Math.ceil` that too?
Comment 5 Patrick Angle 2021-07-06 11:45:32 PDT
Comment on attachment 432948 [details]
Patch v1.0

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

>> Source/WebInspectorUI/UserInterface/Views/NavigationBar.js:226
>> +            return visibleNavigationItems.reduce((total, item) => total + Math.ceil(item.width), 0);
> 
> There's another `navigationItem.width` on :255.  Should we `Math.ceil` that too?

Yes, since we are using `ceil` to calculate the amount we are now subtracting from. This is what I get for having my search be in case-sensitive mode 😅
Comment 6 Patrick Angle 2021-07-06 12:12:21 PDT
Created attachment 432958 [details]
Patch v1.1 - Review notes
Comment 7 EWS 2021-07-06 13:25:56 PDT
Committed r279613 (239434@main): <https://commits.webkit.org/239434@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 432958 [details].