Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update LTW shortcode media queries #163

Merged
merged 4 commits into from
Jul 23, 2020
Merged

Update LTW shortcode media queries #163

merged 4 commits into from
Jul 23, 2020

Conversation

joshdarby
Copy link

Changes

This pull request makes the following changes:

  • Updates the min and max width media queries for the LTW shortcode stylings from 771px and 770px to 981px and 980px

Before on iPhone X:
Screen Shot 2020-07-23 at 9 30 30 AM

After on iPhone X:
Screen Shot 2020-07-23 at 9 30 09 AM

Why

For #162

Testing/Questions

Features that this PR affects:

  • Mobile version of LTW shortcode

Questions that need to be answered before merging:

  • Is this PR targeting the correct branch in this repository?
  • Is this new target size for the media queries ok?

Steps to test this PR:

  1. View a page with the LTW shortcode and make sure the mobile version works as expected

@joshdarby joshdarby added this to the INOV-001 milestone Jul 23, 2020
@joshdarby joshdarby requested a review from benlk July 23, 2020 13:33
@joshdarby joshdarby self-assigned this Jul 23, 2020
@benlk
Copy link
Collaborator

benlk commented Jul 23, 2020

Chrome, with device emulation set to "Nexus 5X", which is an older Android phone:

Screen Shot 2020-07-23 at 12 00 13

@joshdarby
Copy link
Author

@benlk Hm, I wonder if that's from the image? Here's how Nexus 5X looks for me

Screen Shot 2020-07-23 at 12 03 07 PM

@joshdarby
Copy link
Author

Still looks normal to me when adding the image used on prod 😕

Screen Shot 2020-07-23 at 12 06 17 PM

@benlk
Copy link
Collaborator

benlk commented Jul 23, 2020

Both our zoom levels look the same: way too zoomed-out for this content, even after removing the Media and Text block at the top of the page:

Screen Shot 2020-07-23 at 12 05 51

Switching from the "Local That Works" template to "Full Width Page" gives me this:

Screen Shot 2020-07-23 at 12 06 29

@joshdarby
Copy link
Author

@benlk Does adding width: 100% to .current-ltw-shortcode fix the issue for you?

@benlk
Copy link
Collaborator

benlk commented Jul 23, 2020

No:

Screen Shot 2020-07-23 at 12 11 25

I think we're missing a meta value somewhere that sets the screen scaling.

@joshdarby
Copy link
Author

@benlk Oh wow, you're right. We're missing <meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0">

@benlk
Copy link
Collaborator

benlk commented Jul 23, 2020

That's part of Largo's header.php, which template-page-ltw.php misses because we're making a custom header rather than use get_header(). What else might we be missing from that file?

@joshdarby
Copy link
Author

@benlk e822825 reverts the previous media query updates and a306b81 adds in all of the missing meta tags and other header functions that we were missing

Copy link
Collaborator

@benlk benlk left a comment

Choose a reason for hiding this comment

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

With the Local That Works template:

Screen Shot 2020-07-23 at 12 36 37

@joshdarby joshdarby merged commit 464631d into staging Jul 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants