Skip to content

Fix #2812: incorrect width of blog entry author photo #2842

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jul 8, 2017
Merged

Fix #2812: incorrect width of blog entry author photo #2842

merged 2 commits into from
Jul 8, 2017

Conversation

AnEmortalKid
Copy link
Contributor

Fixes #2812

@AnEmortalKid
Update dottydoc.css
ac57b14 
changed author-image width to be auto, since images are 200x200 and they were getting clipped to 100x200

pre/post screenshots: http://imgur.com/a/Td0IN
dottybot
dottybot reviewed Jul 8, 2017
View reviewed changes
Copy link
Member

@dottybot dottybot left a comment

Choose a reason for hiding this comment

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

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Have an awesome day! ☀️

@AnEmortalKid AnEmortalKid changed the title Patch 1 Fix for 2812 incorrect width of blog entry author photo Jul 8, 2017
@smarter
Copy link
Member

smarter commented Jul 8, 2017

Hi, thanks for your contribution which looks good to me! Before we can merge it you'll need to accept the Scala CLA at https://www.lightbend.com/contribute/cla/scala

@AnEmortalKid
Copy link
Contributor Author

@smarter I think the cla bot lied to me!

Hello, and thank you for opening this PR! :tada:

All contributors have signed the CLA, thank you! :heart:

Have an awesome day! :sunny:

Let me get on that. I could try to fix the bot to have the link to the scala CLA (not sure what checks its missing since it still posted even though the check failed)

@AnEmortalKid
Copy link
Contributor Author

Thank you for signing the Scala CLA.

If you have already sent a pull request please add a comment to the pull request saying you did sign the CLA.

We are looking forward to your contributions!

I have signed the CLA

@smarter
Copy link
Member

smarter commented Jul 8, 2017

Yeah the bot is being weird, it's supposed to check for the CLA but gets it wrong for some reason, we haven't had time to figure out why yet. Anyway, thanks for signing the CLA!

smarter
smarter approved these changes Jul 8, 2017
View reviewed changes
Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

LGTM.

@smarter
Copy link
Member

smarter commented Jul 8, 2017 •
edited
Loading

Oh actually, there's one change you could do to make this even better: in one of your commit message start with "Fix #2812: ...", this way github will automatically close the issue when this PR is merged.

@AnEmortalKid AnEmortalKid changed the title Fix for 2812 incorrect width of blog entry author photo Fix #2812: incorrect width of blog entry author photo Jul 8, 2017
@smarter
Copy link
Member

smarter commented Jul 8, 2017

Note that just changing the PR title does not work for github auto-closing feature: you need to actually change the commit message of one of your commit (you can edit the last commit message with git commit --amend and then push the result with git push --force).

@AnEmortalKid
Fix scala#2812: update author-image width
39c34cb 
Fixing author-image to be bound to 100px max. Fixes: scala#2812

With:
```
width:100px
height:auto
```

On firefox, the image is scaled to be 100x100. However on chrome, it ends up being 200x100.

Since we wan't to fix the width to 100px (from that original css), I've updated the max-width and max-height to be 100, so the image is always squared.
@AnEmortalKid
Copy link
Contributor Author

Ah I did not read the "one of your commit messages bit", my bad. Done

@smarter
Copy link
Member

smarter commented Jul 8, 2017

Thanks! I'll merge once the CI confirms the tests pass.

@smarter smarter merged commit 1392444 into scala:master Jul 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dottybot dottybot dottybot left review comments

@smarter smarter smarter approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AnEmortalKid @smarter @dottybot