Skip to content
This repository has been archived by the owner on Oct 6, 2020. It is now read-only.

Add phone outline, re-shoot and process images #130

Closed
wants to merge 1 commit into from

Conversation

rl-king
Copy link
Contributor

@rl-king rl-king commented Dec 4, 2017

This PR fixes #14.

  • Re-took screenshots of all implementations with Chrome on iPhone6-view and processed them all with pngquant.

  • Creates an outline of a generic modern smartphone with 44lines of Scss.

  • Reduces network load on desktop (with images) from 7.2mb to 3.3mb. Which is still not really small. The height of the screenshots could be halved, but this would make them less nice on a hdpi display.

  • One issue with this PR might be that all current pull requests will have the 'wrong' image with a phone in it.

  • If you'd like I'll update the docs on how to take a screenshot

screenshot
Example

@addyosmani
Copy link
Member

Thanks for sending over the set of changes. I think it would make a lot of sense for us to update the documentation so there is some guidance on how to take a screenshot.

@housseindjirdeh do you have any opinions on this direction?

@addyosmani addyosmani self-assigned this Dec 16, 2017
@rl-king
Copy link
Contributor Author

rl-king commented Dec 16, 2017

I think an explanation on how and with what settings you can take a screenshot with Chrome should suffice. Maybe requiring some post processing might be a bit much to ask as it requires someone to install some tools and figure out how they work.

Copy link
Member

@addyosmani addyosmani left a comment

Choose a reason for hiding this comment

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

In general, I'm in favour of this direction as it simplifies the need for every contribution to also submit a phone frame with their screenshot.

I'd like to ask for two changes:

  1. Use an Android frame (perhaps from a Moto G4 or Nexus 5X). As PWAs are currently primarily loaded on Android hardware (looking forward to Service Workers being a default in iOS/Safari in the future!), I think that makes sense.
  2. Add a note about which DevTools emulation preset should be used to get out the right screenshot size (to the readme). Nexus 5X might be a pretty straight-forward option here if we go for this in 1.

@housseindjirdeh
Copy link
Collaborator

Thanks so much for this @rl-king, and apologies on my part for being hands off this project. I completely missed this.

I agree that this will pose an issue with all PRs coming in. @addyosmani and I are currently in the process of going through each of them so once that's done, I'll take this on.

Agreed on having a note for which device preset to use on DevTools. I'll add that as well!

@housseindjirdeh
Copy link
Collaborator

@rl-king thanks for all the help with this mate. I pulled down your branch and finished things up in a separate PR! #157

Will close this in the meantime :) Thanks again for getting the ball rolling with this.

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.

Phone frame for site screenshots
3 participants