Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

bug 817755: Further tweak calendar week event bar height calculation #6864

Merged
merged 1 commit into from

2 participants

@lmorchard
Collaborator

Seems like CSS changes from the overlap stuff made borders & margins apply differently. This tweak to the previous bar height calculation looks like it accounts for it.

ATTN: @lightsofapollo

apps/calendar/js/views/week_child.js
@@ -46,7 +46,7 @@ Calendar.ns('Views').WeekChild = (function() {
*/
_assignHeight: function(element, hoursDuration) {
var percHeight = hoursDuration * 100;
- var pxHeight = hoursDuration * 2;
+ var pxHeight = (hoursDuration * 2) - 5;
@lightsofapollo Owner

ouch! is it possible to calculate this dynamically? I am sure someone will change the css and forget to alter it in the JS.

@lmorchard Collaborator

I tried, but couldn't find a decent way.

I might look again, but I think this comes from the top + bottom margin of the <ol>, bottom border of the <ol>, and top + bottom margin of the <li>. Those are each 1px, so it adds up to 5. It's kind of a chicken-and-egg problem, in that I need to have all those elements available first, and rendered, before I can inspect the styles. And, need to find a spot to find a sample instance to do that once, so I'm not doing it everywhere. (Or maybe cache it)

@lightsofapollo Owner

Damn, caching it is an option but maybe we need to look at structuring the html/css differently so we can avoid these issues...

I am ok with the patch in its current state but we need to mark a big //TODO: fix this so its not hardcoded on that 5px.

@lmorchard Collaborator

Also, now that I try again, I can't seem to get anything for element.style.margin, element.style.marginTop, or element.style.marginBottom... Which I think is because it hasn't been calculated yet, or something *handwaves* not sure

@lightsofapollo Owner

Its a display problem I think, those will not be available until the view is actually displayed...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@lightsofapollo

@lmorchard r=me provided we file a followup bug to remove this hard-coded stuff.

@lmorchard
Collaborator

Updated the commit with a TODO comment on the icky magic tweak

@lightsofapollo lightsofapollo merged commit 1f1a5b0 into mozilla-b2g:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 7, 2012
  1. @lmorchard
This page is out of date. Refresh to see the latest.
View
6 apps/calendar/js/views/week_child.js
@@ -46,7 +46,11 @@ Calendar.ns('Views').WeekChild = (function() {
*/
_assignHeight: function(element, hoursDuration) {
var percHeight = hoursDuration * 100;
- var pxHeight = hoursDuration * 2;
+
+ // TODO: This is a magic calculation based on current CSS. Fix this so
+ // that it can be dynamic based on CSS, or fix CSS to not need this.
+ var pxHeight = (hoursDuration * 2) - 5;
+
element.style.height = 'calc(' + percHeight + '% + ' + pxHeight + 'px)';
},
View
2  apps/calendar/test/unit/views/week_child_test.js
@@ -69,7 +69,7 @@ suite('views/week_child', function() {
subject.date = new Date(2012, 0, 1);
subject._assignPosition(busy, el);
- assert.equal(el.style.height, 'calc(325% + 6.5px)', 'height');
+ assert.equal(el.style.height, 'calc(325% + 1.5px)', 'height');
});
test('#create', function() {
Something went wrong with that request. Please try again.