Closed
Bug 1396085
Opened 7 years ago
Closed 7 years ago
Photon menu killed the Sync calls to action
Categories
(Firefox :: Sync, defect)
Tracking
()
RESOLVED
FIXED
Firefox 57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: rfeeley, Assigned: markh)
Details
Attachments
(3 files)
STEPS TO REPRODUCE Sign in to Sync on only one device Open Synced Tabs panel EXPECTED RESULTS Links should be there link in the sidebar ACTUAL RESULTS They were removed, I believe during Photon migration
Comment 1•7 years ago
|
||
I can look into this and provide some more UX specs... NI'ing myself. Ryan, can you detail which links need to be there and define any special states for the UI? Thanks!
Flags: needinfo?(rfeeley)
Flags: needinfo?(abenson)
Reporter | ||
Comment 2•7 years ago
|
||
Exactly the copy that is there in the sidebar. It used to be there, but it vanished in the new menu.
Flags: needinfo?(rfeeley)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → markh
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Assignee | ||
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8904417 [details] Bug 1396085 - ensure the sync widget is always initialized and animate the sync spinner. https://reviewboard.mozilla.org/r/176228/#review181162 ::: browser/components/customizableui/CustomizableWidgets.jsm:259 (Diff revision 1) > DECKINDEX_NOCLIENTS: 3, > }, > TABS_PER_PAGE: 25, > NEXT_PAGE_MIN_TABS: 5, // Minimum number of tabs displayed when we click "Show All" > onCreated(aNode) { > + this._initialize(aNode); This is a little suspect - I'm not sure what the semantics are for onCreated, but there may be a better way to perform a one-off initialization for the widget?
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8904417 [details] Bug 1396085 - ensure the sync widget is always initialized and animate the sync spinner. https://reviewboard.mozilla.org/r/176228/#review181318 r=me if you unify the selector for the animation. ::: browser/components/customizableui/CustomizableWidgets.jsm:259 (Diff revision 1) > DECKINDEX_NOCLIENTS: 3, > }, > TABS_PER_PAGE: 25, > NEXT_PAGE_MIN_TABS: 5, // Minimum number of tabs displayed when we click "Show All" > onCreated(aNode) { > + this._initialize(aNode); I don't think there is, so this is fine for now. ::: browser/themes/shared/menupanel.inc.css:90 (Diff revision 1) > + animation: syncRotate 0.8s linear infinite; > + fill: #0a84ff; Can you just add this selector to https://dxr.mozilla.org/mozilla-central/source/browser/themes/shared/customizableui/panelUI.inc.css#809 ? We now technically have 2 identically-named keyframes, which doesn't seem good.
Attachment #8904417 -
Flags: review?(gijskruitbosch+bugs) → review+
Comment hidden (mozreview-request) |
Pushed by mhammond@skippinet.com.au: https://hg.mozilla.org/integration/autoland/rev/fc8ba9723756 ensure the sync widget is always initialized and animate the sync spinner. r=Gijs
Comment 8•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/fc8ba9723756
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Updated•7 years ago
|
Flags: needinfo?(abenson)
Reporter | ||
Comment 9•7 years ago
|
||
The link is there now, but the menu height has gone wild.
Assignee: markh → rfeeley
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 10•7 years ago
|
||
Why did you assign this to yourself? Are you actually sure this bug regressed the panel height thing? Generally, IME it's more useful to file a separate followup bug unless it's really clear that the patch in the bug just did nothing good and/or broke the entire world, which I don't think is the case here.
Flags: needinfo?(rfeeley)
Comment 11•7 years ago
|
||
I wonder, for instance, if bug 1374315 caused the situation in attachment 8905161 [details]...
Assignee | ||
Comment 12•7 years ago
|
||
I think Ryan just misunderstood how we use Bugzilla - I opened bug 1397586 for this new issue.
Assignee: rfeeley → markh
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Flags: needinfo?(rfeeley)
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•