New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do not set invalid style #12214
Do not set invalid style #12214
Conversation
e111c62
to
9f5727e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current version did not werk because !important
is not allowed for styles set directly on an element. Which means all the styles set here did not have any effect.
The patch below should work, margin
seems to have no effect on offsetHeight
, though border
does. left
is not needed if the div is removed.
diff --git a/src/ol/render/canvas.js b/src/ol/render/canvas.js
index 61780050a..e5f793324 100644
--- a/src/ol/render/canvas.js
+++ b/src/ol/render/canvas.js
@@ -306,10 +306,10 @@ export const measureTextHeight = (function () {
if (!div) {
div = document.createElement('div');
div.innerHTML = 'M';
- div.style.margin = '0 !important';
- div.style.padding = '0 !important';
- div.style.position = 'absolute !important';
- div.style.left = '-99999px !important';
+ div.style.padding = '0';
+ div.style.border = 'none';
+ div.style.position = 'absolute';
}
div.style.font = fontSpec;
document.body.appendChild(div);
9f5727e
to
c892dc2
Compare
Wow, good find, @MoonE. You're absolutely right, I just force-pushed a commit with your suggestion, and it works indeed. |
c892dc2
to
470fbc2
Compare
The pushed commit isn't quite my suggestion. Anyway, to be extra sure, it might as well be diff --git a/src/ol/render/canvas.js b/src/ol/render/canvas.js
index 61780050a..3d5ea7a0e 100644
--- a/src/ol/render/canvas.js
+++ b/src/ol/render/canvas.js
@@ -306,10 +306,13 @@ export const measureTextHeight = (function () {
if (!div) {
div = document.createElement('div');
div.innerHTML = 'M';
- div.style.margin = '0 !important';
- div.style.padding = '0 !important';
- div.style.position = 'absolute !important';
- div.style.left = '-99999px !important';
+ div.style.minHeight = '0';
+ div.style.maxHeight = 'none';
+ div.style.height = 'auto';
+ div.style.padding = '0';
+ div.style.border = 'none';
+ div.style.position = 'absolute';
+ div.style.display = 'block';
}
div.style.font = fontSpec;
document.body.appendChild(div);
Might be hyper correct now. |
|
470fbc2
to
7ec5cbf
Compare
Thanks, looks good to me now. Do you agree, @MoonE ? |
We need to use block display for measuring text heights.
Fixes #12213.