Issue metadata
Sign in to add a comment
|
Interstitial buttons are on bottom of page |
||||||||||||||||||||||||
Issue descriptionAt certain desktop window sizes, the interstitial buttons are jumping to the bottom of the page. It looks like it's a problem with the logic around small-window sizing. The bug is in Edward's CL that landed in M60, and I don't want to revert it. Instead we should fix and then merge the fix to 60.
,
May 27 2017
Hmmm, I don't believe I changed the CSS in a way that affects the button positioning in my recent CL. It's more likely that this was something that I didn't pick up in this CL submitted a few weeks ago which simplified the media queries a lot: https://codereview.chromium.org/2788323002/ The media queries will need adjusting. There's a middling space that tablets and smaller desktop screens both occupy. If someone wants to take a look before I'm back it's in: @media (max-width: 420px) and (min-height: 401px) and (min-width: 240px), (max-height: 736px) and (min-height: 240px) and (min-width: 421px) body .nav-wrapper {}
,
May 29 2017
I took a quick look on Friday and came to the same conclusion. I think we just need to adjust the media queries. Surprised no one has complained! I'm OOO through June 6 & didn't have time to land a fix before Friday evening. Nate, could you please handle this? We should probably get a fix merged back quickly.
,
May 30 2017
Was OOO on Friday, but I'll take a look today and see how we can adjust the media queries.
,
May 31 2017
Pending CL is up at https://chromium-review.googlesource.com/c/518544/, just waiting for Edward Jung to be back for UI comments.
,
Jun 2 2017
,
Jun 5 2017
Thanks Nate, lgtm'd your CL.
,
Jun 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a44afd2d732a3192329ea398ec030eab6cfe3aeb commit a44afd2d732a3192329ea398ec030eab6cfe3aeb Author: Nate Fischer <ntfschr@chromium.org> Date: Mon Jun 05 20:55:03 2017 Choose desktop interstitial layout for medium views We should choose the desktop layout over the mobile layout interstitials for middle-sized views, since we're switching to the desktop layout too soon. This CL changes the threshold from height = 736px to 560px and removes the extra margin on the details element. Together, these changes help improve transitions between mobile and desktop: - We use the desktop layout for more middle view sizes, and we can display more of the details because we're wasting less space with padding - When we switch to mobile layout, the "Details" button is in nearly the same vertical spot as it would've been in desktop layout (it only moves 35px lower). - Clicking "details" in the desktop layout usually shows at least a tiny bit of the details on the screen. This serves as a visual cue that the user needs to scroll to see all the content. Otherwise, the "details" button looks as if it doesn't do anything. Test: Manual - compared screenshots Bug: 726913 Change-Id: I326047767da1660eb5a9dc272551676819eaaa82 Reviewed-on: https://chromium-review.googlesource.com/518544 Commit-Queue: Nate Fischer <ntfschr@chromium.org> Reviewed-by: Nathan Parker <nparker@chromium.org> Cr-Commit-Position: refs/heads/master@{#477080} [modify] https://crrev.com/a44afd2d732a3192329ea398ec030eab6cfe3aeb/components/security_interstitials/core/browser/resources/interstitial_v2.css [modify] https://crrev.com/a44afd2d732a3192329ea398ec030eab6cfe3aeb/components/security_interstitials/core/browser/resources/interstitial_v2_mobile.js
,
Jun 5 2017
,
Jun 5 2017
,
Jun 6 2017
Please tag with appropriate OSs.
,
Jun 6 2017
Technically affects all platforms, but due to screen dimensions this is only an issue for desktop platforms.
,
Jun 6 2017
Your change meets the bar and is auto-approved for M60. Please go ahead and merge the CL to branch 3112 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jun 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6bcf1f9a352f42fb145a11c214439ff0567e28a8 commit 6bcf1f9a352f42fb145a11c214439ff0567e28a8 Author: Nate Fischer <ntfschr@chromium.org> Date: Tue Jun 06 21:27:40 2017 Choose desktop interstitial layout for medium views We should choose the desktop layout over the mobile layout interstitials for middle-sized views, since we're switching to the desktop layout too soon. This CL changes the threshold from height = 736px to 560px and removes the extra margin on the details element. Together, these changes help improve transitions between mobile and desktop: - We use the desktop layout for more middle view sizes, and we can display more of the details because we're wasting less space with padding - When we switch to mobile layout, the "Details" button is in nearly the same vertical spot as it would've been in desktop layout (it only moves 35px lower). - Clicking "details" in the desktop layout usually shows at least a tiny bit of the details on the screen. This serves as a visual cue that the user needs to scroll to see all the content. Otherwise, the "details" button looks as if it doesn't do anything. TBR=ntfschr@chromium.org (cherry picked from commit a44afd2d732a3192329ea398ec030eab6cfe3aeb) Test: Manual - compared screenshots Bug: 726913 Change-Id: I326047767da1660eb5a9dc272551676819eaaa82 Reviewed-on: https://chromium-review.googlesource.com/518544 Commit-Queue: Nate Fischer <ntfschr@chromium.org> Reviewed-by: Nathan Parker <nparker@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#477080} Reviewed-on: https://chromium-review.googlesource.com/526477 Reviewed-by: Nate Fischer <ntfschr@chromium.org> Cr-Commit-Position: refs/branch-heads/3112@{#211} Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897} [modify] https://crrev.com/6bcf1f9a352f42fb145a11c214439ff0567e28a8/components/security_interstitials/core/browser/resources/interstitial_v2.css [modify] https://crrev.com/6bcf1f9a352f42fb145a11c214439ff0567e28a8/components/security_interstitials/core/browser/resources/interstitial_v2_mobile.js
,
Jun 6 2017
,
Jun 8 2017
Verified the issue on windows 10, Mac 10.12.5 and Ubuntu 14.04 using chrome beta version #60.0.3112.24 as per comment #0. Observed that interstitial buttons are not seen at bottom of page after resizing browser window as seen in the screen shot in comment #0. Hence, the fix is working as expected. Attaching screen cast and screen shot for reference. Hence, adding the verified labels. Thanks...!!
,
Aug 24
Bulk edit: marking stale 'fixed' bugs as 'verified' since they don't need verification at this point. |
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by f...@chromium.org
, May 26 2017