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 background-size example #339

Merged
merged 5 commits into from Nov 21, 2017
Merged

Conversation

wbamberg
Copy link
Contributor

@wbamberg wbamberg commented Nov 19, 2017

Here are some updates to the background-size example.

The original example is too tall to fit without scrolling, so I've reduced the vertical height to prevent that:

  • removed comments: I think the code is pretty self-explanatory, and given how precious vertical space is, we should avoid comments unless they are really needed

  • removed one of the examples, which covered quite similar ground to another one

I've also chosen a different image: I chose this for a few reasons:

  • the aspect seems like it will work better to show the difference between keywords like cover and contain

  • the shape is very simple and recognizable, so distortions when you set width and height are more obvious

I'm happy to talk about these changes though if you don't agree.

Copy link
Contributor

@schalkneethling schalkneethling left a comment

Choose a reason for hiding this comment

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

I've also chosen a different image

Works for me. I am wondering though if perhaps we want to add all the images used in examples inside a examples folder inside media to keep em separated ;p Also, its worth running these images through either ImageOptim or http://tinypng.com

@wbamberg
Copy link
Contributor Author

wbamberg commented Nov 20, 2017

Works for me. I am wondering though if perhaps we want to add all the images used in examples inside a examples folder inside media to keep em separated ;p Also, its worth running these images through either ImageOptim or http://tinypng.com

These should be done now. Good call on moving the images, it did occur to me that this would get messy, but I didn't follow the thought through.

@schalkneethling schalkneethling merged commit 0d1c319 into mdn:master Nov 21, 2017
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.

None yet

2 participants