Closed
Bug 1379208
Opened 8 years ago
Closed 8 years ago
Positioning the search tooltips should not need to flush layout
Categories
(Firefox :: Settings UI, enhancement, P1)
Firefox
Settings UI
Tracking
()
RESOLVED
DUPLICATE
of bug 1363730
People
(Reporter: jaws, Unassigned)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
Bug 1351867 implemented showing the search tooltips but it uses getBoundingClientRect to figure out where to absolutely position the tooltip.
http://searchfox.org/mozilla-central/rev/f1472e5b57fea8d4a7bbdd81c44e46958fe3c1ce/browser/components/preferences/in-content-new/findInPage.js#434
We remove this code and instead just add the following CSS styles to .search-tooltip:
top: -55%;
left: 0;
right: 0;
width: -moz-fit-content;
margin-left: auto;
margin-right: auto;
Reporter | ||
Comment 1•8 years ago
|
||
Actually this positions the tooltip when searching for "container" in the center when it is expected to be above the button on the right side. I'll need to figure something else out, and I assume there are cases where top:0; isn't the right thing either.
If we could just use ::before on the anchor element instead of creating a new adjacent node that has to be positioned absolutely relative to their common ancestor.
Reporter | ||
Comment 2•8 years ago
|
||
I've hit an impasse working on this and it comes back to bug 1363730 so I'm going to close this and dupe it to bug 1363730.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
Reporter | ||
Updated•8 years ago
|
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Comment hidden (mozreview-request) |
Reporter | ||
Updated•8 years ago
|
Assignee: nobody → jaws
Comment 4•8 years ago
|
||
Tooltip position seems wrong after applying the patch.
Updated•8 years ago
|
Status: REOPENED → ASSIGNED
Flags: qe-verify-
Priority: -- → P1
Whiteboard: [photon-preference][triage]
Comment 5•8 years ago
|
||
mozreview-review |
Comment on attachment 8885024 [details]
Bug 1379208 - Position the tooltips using only one layout flush and no modifications to the surrounding DOM of the tooltip nodes.
https://reviewboard.mozilla.org/r/155850/#review161036
Jared,
Thanks for helping work on this. Without pseudo element and postion: abolution support for XUL element, the current workaround causes so many regressions and incorrect tooltip position issues. I'd like this patch and reduce layout flush as much as possible.
Please see my attachment. I saw that patch broke tooltip position.
Attachment #8885024 -
Flags: review?(rchien) → review-
Comment 6•8 years ago
|
||
After playing with it for a while, it seems like still buggy such as hover mouse on tooltip is unable to see the opacity effect unless you hover on button itself.
My conclusion is that with `position: fixed` solution treating the pseudo element as a part of the anchor element, so tooltip will not be an independent element and support the hover effect as expected.
Reporter | ||
Comment 7•8 years ago
|
||
Well it was worth a shot and I learned something new about transform and position:fixed :)
Assignee: jaws → nobody
Status: ASSIGNED → RESOLVED
Closed: 8 years ago → 8 years ago
Resolution: --- → DUPLICATE
Comment 8•8 years ago
|
||
mozreview-review |
Comment on attachment 8885024 [details]
Bug 1379208 - Position the tooltips using only one layout flush and no modifications to the surrounding DOM of the tooltip nodes.
https://reviewboard.mozilla.org/r/155850/#review161294
Attachment #8885024 -
Flags: review?(mconley)
Comment 9•8 years ago
|
||
remove whiteboard tag due to its DUPLICATE
Whiteboard: [photon-preference][triage]
You need to log in
before you can comment on or make changes to this bug.
Description
•