Closed Bug 1380738 Opened 7 years ago Closed 7 years ago

Fix the size of the avatar to stop reflow

Categories

(Firefox :: Sync, defect, P2)

55 Branch
defect

Tracking

()

RESOLVED FIXED
Firefox 56
Tracking Status
firefox56 --- fixed

People

(Reporter: rfeeley, Assigned: lyret, Mentored)

Details

(Keywords: good-first-bug)

Attachments

(2 files)

Attached image flicker.gif
Sounds like a simple maxwidth to width fix

STEPS TO REPRODUCE
1. Sign in to Sync
2. Go to Sync prefs
3. Restart browser
4. Open a web page
5. Resize window (possibly optional)
6. Switch to sync prefs

EXPECTED RESULTS
- Firefox Accounts badge is fixed size

ACTUAL RESULTS
- Firefox Accounts badge reflows
Mentor: eoger
Keywords: good-first-bug
Priority: -- → P2
Hi,

I would like to get this bug assigned on my name so that I can fix it.

Thanks.
Flags: needinfo?(eoger)
Hi,

We typically assign a bug to a contributor once a patch has been posted.
Flags: needinfo?(eoger)
Hi,
I am a beginner, can you please guide me a bit so that I can make contributions to this project.
So first you want to be able to build Firefox in artifact mode. This will help you getting started:
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build

Once you have built Firefox, here's what this bug is about:

In the Sync preferences page, open the inspector, try to find the avatar element (it's ID is fxaProfileImage) and disable the list-style-image rules.
This will put you in the same state as the gif Ryan linked.

What needs to be done is to replace the min-width rule a width rule, in order to keep the profile image size constant so we avoid reflows. I also noticed that the height of the avatar is not encoded either! You will need to add that.
When Sync is unconfigured, the encoded min-width rule is different (60px), you will have to change it and also add height.
Comment on attachment 8889949 [details]
Bug 1380738 - Fix the size of the avatar to avoid reflow.

https://reviewboard.mozilla.org/r/161014/#review166472

Almost there, thank you!

::: commit-message-32d9d:1
(Diff revision 1)
> +Bug 1380738 - Fix the size of the avatar to avoid reflow. r=eoger

Nitpick: change r=eoger to r?eoger. When you commit this is changed to a = automatically.

::: browser/themes/shared/incontentprefs-old/preferences.inc.css:395
(Diff revision 1)
>    image-rendering: auto;
>    border: 1px solid transparent;
>  }
>  
>  #fxaLoginStatus[hasName] #fxaProfileImage {
>    max-width: 80px;

You will need to change this too.

::: browser/themes/shared/incontentprefs-old/preferences.inc.css:522
(Diff revision 1)
>    word-break: break-all;
>  }
>  
>  .fxaFirefoxLogo {
>    list-style-image: url(chrome://browser/skin/fxa/logo.png);
> -  max-width: 64px;
> +  width: 60px;

Keep this, but at 64px (good catch! I thought we only had 2 rules)
Attachment #8889949 - Flags: review?(eoger)
Assignee: nobody → lyretandrpg
Status: NEW → ASSIGNED
Thanks, however you still need to address this comment: https://reviewboard.mozilla.org/r/161014/#comment215554
Comment on attachment 8889949 [details]
Bug 1380738 - Fix the size of the avatar to avoid reflow.

https://reviewboard.mozilla.org/r/161014/#review166562

::: browser/themes/shared/incontentprefs-old/preferences.inc.css:395
(Diff revisions 2 - 3)
>    image-rendering: auto;
>    border: 1px solid transparent;
>  }
>  
>  #fxaLoginStatus[hasName] #fxaProfileImage {
> -  max-width: 80px;
> +  max-width: 60px;

The value (80px) is actually fine, what needs needs to be done is the same as the other similar rules: change it to width, add height.
Attachment #8889949 - Flags: review?(eoger)
Comment on attachment 8889949 [details]
Bug 1380738 - Fix the size of the avatar to avoid reflow.

https://reviewboard.mozilla.org/r/161014/#review166568

Awesome thanks!
Attachment #8889949 - Flags: review?(eoger) → review+
Pushed by eoger@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ceca80096ef1
Fix the size of the avatar to avoid reflow. r=eoger
https://hg.mozilla.org/mozilla-central/rev/ceca80096ef1
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
You need to log in before you can comment on or make changes to this bug.