Skip to content
This repository was archived by the owner on Apr 3, 2019. It is now read-only.

Conversation

@hritvi
Copy link
Contributor

@hritvi hritvi commented Mar 16, 2019

fixes #5313

It now looks like this on mobile view:
Screenshot from 2019-03-16 15-40-51

@ryanfeeley Please review.

@ryanfeeley
Copy link
Contributor

@hritvi Looks fantastic! As the designer, I'm not sure if I should be approving the code here. @shane-tomlinson ?

@lmorchard lmorchard self-assigned this Mar 18, 2019
@lmorchard
Copy link
Contributor

lmorchard commented Mar 18, 2019

I can take a peek here!

Edit: Hmm, actually maybe not. I don't have an iOS device, and I'm having trouble reproducing the original issue with a rotated photo. I'm not sure how the CSS here would address that - or how the original CSS would have caused it.

I'm wondering if there's a further issue with how we're actually handling the image and any rotation info in EXIF data. (Kind of guessing here)

@lmorchard
Copy link
Contributor

lmorchard commented Mar 18, 2019

Yeah, I think I'm super confused by this issue. I found a tall sample image with different EXIF data values and didn't see the squash or rotation from #5313. Also looks like things are a little off, at least where I'm trying it on Linux

Master:
Capture

This PR:
Capture2

@hritvi
Copy link
Contributor Author

hritvi commented Mar 19, 2019

@lmorchard I think so the squash and rotation errors were resolved by some other PR as even I was unable to replicate that. There was another issue, that is #7045, which was closed as a dup of #5313 and it stated about the oversizing of profile images.
Therefore this PR only addresses the oversizing of profile images. zcould you now take a look at it please. Thanks 🙂

@lmorchard
Copy link
Contributor

Well, I didn't see the rotation & squash issues, but I did see an issue in this PR with the image being off-center in my screenshots. Not sure what might be causing that - maybe the left: 0 here? That doesn't seem like it should cause a problem though

@shane-tomlinson
Copy link

This repo has been deprecated and migrated to https://github.com/mozill/fxa. Please open this PR against that repo.

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.

4 participants