Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(118)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: ui/webui/resources/cr_elements/cr_action_menu/cr_action_menu.js

Issue 2951703002: [cr-action-menu] Fix anchoring to offscreen elements. (Closed)
Patch Set: address comments Created 3 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« chrome/test/data/webui/cr_elements/cr_action_menu_test.js ('K') | « chrome/test/data/webui/cr_elements/cr_action_menu_test.js ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: ui/webui/resources/cr_elements/cr_action_menu/cr_action_menu.js
diff --git a/ui/webui/resources/cr_elements/cr_action_menu/cr_action_menu.js b/ui/webui/resources/cr_elements/cr_action_menu/cr_action_menu.js
index 854a812d00a94af6babd12cf556b80d0f8735000..36c742341a56fc2b56a739908f07dcc38fdaa87e 100644
--- a/ui/webui/resources/cr_elements/cr_action_menu/cr_action_menu.js
+++ b/ui/webui/resources/cr_elements/cr_action_menu/cr_action_menu.js
@@ -6,14 +6,14 @@
* @typedef {{
* top: number,
* left: number,
- * width: (number| undefined),
- * height: (number| undefined),
- * anchorAlignmentX: (number| undefined),
- * anchorAlignmentY: (number| undefined),
- * minX: (number| undefined),
- * minY: (number| undefined),
- * maxX: (number| undefined),
- * maxY: (number| undefined),
+ * width: (number|undefined),
+ * height: (number|undefined),
+ * anchorAlignmentX: (number|undefined),
+ * anchorAlignmentY: (number|undefined),
+ * minX: (number|undefined),
+ * minY: (number|undefined),
+ * maxX: (number|undefined),
+ * maxY: (number|undefined),
* }}
*/
var ShowConfig;
@@ -73,7 +73,6 @@ function getStartPointWithAnchor(
return startPoint;
}
-
/**
* @private
* @return {!ShowConfig}
@@ -86,10 +85,10 @@ function getDefaultShowConfig() {
width: 0,
anchorAlignmentX: AnchorAlignment.AFTER_START,
anchorAlignmentY: AnchorAlignment.AFTER_START,
- minX: 0,
- minY: 0,
- maxX: window.innerWidth,
- maxY: window.innerHeight,
+ minX: document.body.scrollLeft,
+ minY: document.body.scrollTop,
+ maxX: document.body.scrollLeft + window.innerWidth,
+ maxY: document.body.scrollTop + window.innerHeight,
};
}
@@ -252,19 +251,48 @@ Polymer({
/**
* Shows the menu anchored to the given element.
* @param {!Element} anchorElement
+ * @param {ShowConfig=} opt_config
*/
- showAt: function(anchorElement) {
+ showAt: function(anchorElement, opt_config) {
this.anchorElement_ = anchorElement;
+ // Scroll the anchor element into view so that the bounding rect will be
+ // accurate for where the menu should be shown.
this.anchorElement_.scrollIntoViewIfNeeded();
+
+ // Save the scroll position that ensures the anchor element is onscreen.
+ var scrollLeft = document.body.scrollLeft;
+ var scrollTop = document.body.scrollTop;
+
+ // Reset position so that layout isn't affected by the previous position,
+ // and so that the dialog is positioned at the top-start corner of the
+ // document.
+ this.resetStyle_();
dpapad 2017/06/22 21:18:37 After patching this CL locally I discovered an und
calamity 2017/06/28 04:00:47 Changed to use document.scrollingElement instead o
+
+ // Show the dialog which will focus the top-start of the body. This makes
+ // the client rect calculation relative to the top-start of the body.
+ this.showModal();
+
var rect = this.anchorElement_.getBoundingClientRect();
- this.showAtPosition({
- top: rect.top,
- left: rect.left,
- height: rect.height,
- width: rect.width,
- // Default to anchoring towards the left.
- anchorAlignmentX: AnchorAlignment.BEFORE_END,
- });
+ this.positionDialog_(/** @type {ShowConfig} */ (Object.assign(
+ {
+ top: rect.top,
+ left: rect.left,
+ height: rect.height,
+ width: rect.width,
+ // Default to anchoring towards the left.
+ anchorAlignmentX: AnchorAlignment.BEFORE_END,
+ minX: scrollLeft,
+ minY: scrollTop,
+ maxX: scrollLeft + window.innerWidth,
+ maxY: scrollTop + window.innerHeight,
+ },
+ opt_config)));
+
+ // Restore the scroll position.
+ document.body.scrollTop = scrollTop;
+ document.body.scrollLeft = scrollLeft;
+
+ this.addCloseListeners_();
},
/**
@@ -294,6 +322,24 @@ Polymer({
* @param {!ShowConfig} config
*/
showAtPosition: function(config) {
+ this.resetStyle_();
+ this.showModal();
+ this.positionDialog_(config);
+ this.addCloseListeners_();
+ },
+
+ /** @private */
+ resetStyle_: function() {
+ this.style.left = '';
+ this.style.right = '';
+ this.style.top = '0';
+ },
+
+ /**
+ * @param {!ShowConfig} config
+ * @private
+ */
+ positionDialog_: function(config) {
var c = Object.assign(getDefaultShowConfig(), config);
var top = c.top;
@@ -301,20 +347,6 @@ Polymer({
var bottom = top + c.height;
var right = left + c.width;
- this.boundClose_ = this.boundClose_ || function() {
- if (this.open)
- this.close();
- }.bind(this);
- window.addEventListener('resize', this.boundClose_);
- window.addEventListener('popstate', this.boundClose_);
-
- // Reset position to prevent previous values from affecting layout.
- this.style.left = '';
- this.style.right = '';
- this.style.top = '';
-
- this.showModal();
-
// Flip the X anchor in RTL.
var rtl = getComputedStyle(this).direction == 'rtl';
if (rtl)
@@ -324,7 +356,7 @@ Polymer({
left, right, this.offsetWidth, c.anchorAlignmentX, c.minX, c.maxX);
if (rtl) {
- var menuRight = window.innerWidth - menuLeft - this.offsetWidth;
+ var menuRight = document.body.scrollWidth - menuLeft - this.offsetWidth;
this.style.right = menuRight + 'px';
} else {
this.style.left = menuLeft + 'px';
@@ -334,5 +366,17 @@ Polymer({
top, bottom, this.offsetHeight, c.anchorAlignmentY, c.minY, c.maxY);
this.style.top = menuTop + 'px';
},
+
+ /**
+ * @private
+ */
+ addCloseListeners_: function() {
+ this.boundClose_ = this.boundClose_ || function() {
+ if (this.open)
+ this.close();
+ }.bind(this);
+ window.addEventListener('resize', this.boundClose_);
+ window.addEventListener('popstate', this.boundClose_);
+ },
});
})();
« chrome/test/data/webui/cr_elements/cr_action_menu_test.js ('K') | « chrome/test/data/webui/cr_elements/cr_action_menu_test.js ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698