LayoutTests/fast/css-grid-layout/grid-container-width-should-include-scroll-bar-width.html - Issue 535913002: Add scrollbar logical width while computing intrinsic logical width

Side by Side Diff

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Side by Side Diff: LayoutTests/fast/css-grid-layout/grid-container-width-should-include-scroll-bar-width.html

Issue 535913002: Add scrollbar logical width while computing intrinsic logical width (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master

Patch Set: Fix typo + Remove Scrollbar calculation from testcase Created 6 years, 3 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« no previous file with comments | « no previous file | LayoutTests/fast/css-grid-layout/grid-container-width-should-include-scroll-bar-width-expected.txt » ('j') | Source/core/rendering/RenderGrid.cpp » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <!DOCTYPE html>

	2 <html>

	3 <link href="resources/grid.css" rel="stylesheet">

	4 <style>

	5 .grid {

	6 position:absolute;

	7 overflow:scroll;

	8 }

	9 .gridItem {

	10 width: 100px;

	11 height: 100px;

	12 background:green;

	13 }

	14 </style>

	15 <script src="../../resources/check-layout.js"></script>

	16 <body onload="checkLayout('.grid')">

	17

	18 Test that scrollbar width is added to the width of the grid container.

	19 <div class='grid' style='top: 100px' data-expected-width='115'>
	Julien - ping for review 2014/09/10 16:06:54 I don't think we need the position: absolute and t I don't think we need the position: absolute and the top/left offsets in both grid to test data-expected-width so we should remove them. Sunil Ratnu 2014/09/11 10:59:44 I think we need it because when I tried testing af I think we need it because when I tried testing after removing "position: absolute", the whole div (in our case the container) occupies the screen space, and the scrollbar appears at the end, and the data-expected-width comes 784 (http://jsbin.com/harezasuhepu/1/edit). But when we use the absolute positioning, it is seen clearly seen that scrollbar width gets added in the container size. Julien - ping for review 2014/11/19 23:28:40 You don't need position per se. What you want is Show quoted text On 2014/09/11 10:59:44, Sunil Ratnu wrote: > I think we need it because when I tried testing after removing "position: > absolute", the whole div (in our case the container) occupies the screen space, > and the scrollbar appears at the end, and the data-expected-width comes 784 > (http://jsbin.com/harezasuhepu/1/edit). But when we use the absolute > positioning, it is seen clearly seen that scrollbar width gets added in the > container size. You don't need position per se. What you want is a consistent size for the grid container, which can be achieve in other ways. One is to make it shrink-wrap (which is what position: absolute gives you instead of width: auto being sized after your containing block's content width): .grid { width: fit-content; } Another more simple way is just to set the width and height of the grid: .grid { width: 100px; height: 100px; } (your children are just a proxy for getting a size as you're shrink-wrap) The 2nd option is the most appealing for me as it's the most simple too, especially if we remove the .gridItem that are distracting readers from what the test stresses.
	20 <span class='gridItem'></span>

	21 </div>

	22

	23 <div class='grid' style='top: 100px; left: 150px; overflow-y: hidden' data-expec ted-width='100'>

	24 <span class='gridItem'></span>

	25 </div>

	26

	27 </body>

	28 </html>

OLD	NEW