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

feat(driver): add some iphone viewport #8624

Merged
merged 4 commits into from Oct 13, 2020
Merged

feat(driver): add some iphone viewport #8624

merged 4 commits into from Oct 13, 2020

Conversation

ryota-murakami
Copy link
Contributor

@ryota-murakami ryota-murakami commented Sep 18, 2020

User facing changelog

  • Added viewport preset of iphone-7, iphone-8, iphone-se2

Additional details

sake: improve Developer Experience

viewport preset consider useful most people because people tend to memorize mobile device naming e.g. google-pixel, samsung galaxy etc clearly more than device pixels number.

So, This time I added 3 devices that has exact same resolution as a iphone-6(750 x 1334)
source: https://devhints.io/resolutions source hosted github repo over 9,000 and maintain frequency. Seems trusted.

this is draft pull request

In the first place I don't know Cypress dev team has positive feeling about support more viewport,

so I started super low regression risk part.

If Cypress dev team had agree with positive, I'll add some popular devices viewport e.g. google-pixel series, galaxy s20, ipad-pro, ipad-air, series, ipad-3, ipad-4, ipad-5, ipad-6, ipad-7, ipad-8(modern days, call that ipad-8gen generation style also popular on net news, SNS etc.

How has the user experience changed?

Cypress user can use familiar mobile device screen size easily, less research time about cypress viewport preset detail.

Unlike me, I have iphone-7 in real life so I attempt use it viewport, but I can't figure out next number of iphone-7... 🤔❓😭

PR Tasks

Every feedback welcome, I'm glad got comments some thought when you have plenty time and your smartphone doesn't exist in viewport. 😅

Thank you for reading about this! 🤗

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Sep 18, 2020

Thanks for taking the time to open a PR!

@CLAassistant
Copy link

CLAassistant commented Sep 18, 2020

CLA assistant check
All committers have signed the CLA.

@jennifer-shehane
Copy link
Member

@ryota-murakami I see this is still in Draft, is this ready for review or will you have time to finish this PR?

@ryota-murakami ryota-murakami marked this pull request as ready for review October 7, 2020 10:43
@ryota-murakami
Copy link
Contributor Author

@jennifer-shehane Thank you mentioning it, It's ready for review so I just update PR status from Draft!

Copy link
Member

@jennifer-shehane jennifer-shehane left a comment

Choose a reason for hiding this comment

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

Can you update the types to reflect the new presets? https://github.com/cypress-io/cypress/blob/develop/cli/types/cypress.d.ts#L5298:L5298

Also, could you open a pull request against our docs to document the new viewport presets?

@ryota-murakami
Copy link
Contributor Author

@jennifer-shehane

Also, could you open a pull request against our docs to document the new viewport presets?

Sure, I'll update docs viewport presets section and raise PR! 😀

@jennifer-shehane
Copy link
Member

@ryota-murakami I think for future new viewport sizes, we're thinking to pull in some outside dependency where we can request viewports / sizes and use those as the available viewports. Because keeping up with an arbitrary list that people add to just might get out of hand. Just so that answers your question about newer viewports. Maybe there's a library that lists those we can pull in.

That or maybe there's a way to reference or pull in Google Chrome device sizes that they use in emulation mode. Ideally our viewports would match those.

Screen Shot 2020-10-09 at 2 26 39 PM

@ryota-murakami
Copy link
Contributor Author

Also, could you open a pull request against our docs to document the new viewport presets?

@jennifer-shehane I send PR to Docs repo that added exact same iphone medels.
cypress-io/cypress-documentation#3229

@ryota-murakami
Copy link
Contributor Author

@jennifer-shehane Thank you for your answer about viewport direction.

Maybe there's a library that lists those we can pull in.

That's might be cool, I'm looking forward to that! 🤗

@jennifer-shehane jennifer-shehane merged commit 66d1a52 into cypress-io:develop Oct 13, 2020
jennifer-shehane added a commit to cypress-io/cypress-documentation that referenced this pull request Oct 13, 2020
* reflect PR#8624 change to Docs

@close cypress-io/cypress#8624 (review)

* rearrange viewports

Co-authored-by: Jennifer Shehane <jennifer@cypress.io>
@ryota-murakami ryota-murakami deleted the add-iphone-viewport branch October 13, 2020 13:12
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Oct 14, 2020

Released in 5.4.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v5.4.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Oct 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants