1713130 - Restructure Day and Week views using HTML

Status: RESOLVED FIXED

(Calendar :: Calendar Frontend, task)

Description

[Henry Wilkes (they/them) [:henry-x]]

The Calendar Day and Week views could be semantically improved by switching them away from a collection of disconnected XUL <box>es.

Some other benefits:

• Improving the representation in the accessibility tree (if you look at the current representation you'll see what I mean).
• Good opportunity to fix support for right-to-left languages (Bug 1712942). Also, make the rotated view right-to-left as well.
• Get rid of more XUL.

Suggested Structure

Something like

<!-- Hour header column -->
<div aria-hidden="true">
  <!-- Empty corner space -->
  <!-- All-day header? -->
  <!-- 00:00 Hour header -->
  <!-- 01:00 Hour header -->
  ...
</div>
<ol>
  <!-- Day of the week column -->
  <li>
    <h2><!-- Day of the week header --></h2>
    <section>
      <h3><!-- Screen-reader-only -->All Day</h3>
      <ol><!-- Are all-day events ordered? -->
        <li><calendar-editable-item /></li>
        ...
      </ol>
   </section>
    <section>
      <div aria-hidden="true">
        <!-- Empty 00:00 hour box for grid display
           - and clicking to create new events -->
        ...
      </div>
      <h3><!-- Screen-reader-only -->Schedule</h3>
      <ol><!-- Ordered as they are in the Multiweek and Month views. -->
        <li>
          <calendar-event-box>
            <!-- Screen-reader-only description of start and end time. Examples:
               - Starts at 12:30, Ends at 15:15
               - Starts at 12:30, Continues to the next day
               - Continues from the previous day, Ends at 15:15 -->
            <!-- Other details -->
          </calendar-event-box>
        </li>
        ...
      </ol>
    </section>
  </li>
  <!-- Day of the week column -->
  <li><!-- Next day --></li>
  ...
</ol>

For the Day view, we can remove the outer <ol>. For the Week view, I'm not sure whether the outer <ol> is particularly helpful, or whether successive <section>s would be more useful. I'm also not sure about the <h2> and <h3> numbers; I just avoided <h1>.

I think the semantics of this approach is close to what people reading the table are looking for, and that this would translate well into the accessibility tree.

I was going to use a CSS display: grid with lots of display: contents to make this structure appear the same as it is now. Plus, it will be possible to rotate the view as you can now.

Using a HTML table

Note, I orginally considered using a HTML <table>, where each cell corresponds to an hour window, and the items are placed within the cell where they start. However, this does not translate as well to the accessibility tree. Specifically:

• The header would indicate the hour at which the event starts, but you would still be missing the minutes past the hour that the event starts.
• Events that continue from the previous day will appear as if they start at midnight.
• There is no information about when the event ends. In particular, there isn't a clear way to know that an event also extends to the next hour.
• There would be lots of cells with no content.

Behaviour that needs to be preserved

A list of the current behaviour that needs to be preserved and checked (please let me know about any more):

• Dragging and dropping events.
• Dragging to create an event.
• Resizing events.
• Time indicator.
• Event creation through double click or context menu.

Comment 1

[Alessandro Castellani [:aleca]]

For the Day view, we can remove the outer <ol>. For the Week view, I'm not sure whether the outer <ol> is particularly helpful, or whether successive <section>s would be more useful. I'm also not sure about the <h2> and <h3> numbers; I just avoided <h1>.

I can help in testing this on Windows with a screen reader and see which one is semantically more "correct", but overall I think your approach is good.

I was going to use a CSS display: grid with lots of display: contents to make this structure appear the same as it is now. Plus, it will be possible to rotate the view as you can now.

+1

Comment 2

[Geoff Lankow (:darktrojan)]

Looks good so far. How will you handle multiple overlapping events?

Are all-day events ordered?

Theoretically, yes, all-day events crossing multiple days are supposed to be ordered but I'm not sure off the top of my head that they are in this view. I'm also working to have events in the same order as the calendars they're in if they're otherwise the same, but that doesn't really matter from an a11y POV.

For the Day view, we can remove the outer <ol>. For the Week view, I'm not sure whether the outer <ol> is particularly helpful, or whether successive <section>s would be more useful.

The day view is just the week view with only one day showing. (Also the week view doesn't have to display all seven days.) Might as well keep it simple.

Behaviour that needs to be preserved

• Dragging to create an event.

Comment 3

[Henry Wilkes (they/them) [:henry-x]]

(In reply to Alessandro Castellani [:aleca] from comment #1)

I can help in testing this on Windows with a screen reader and see which one is semantically more "correct", but overall I think your approach is good.

Thanks, that will be helpful.

(In reply to Geoff Lankow (:darktrojan]) from comment #2)

Dragging to create an event.

Did not know about this. Added it to the list.

Comment 4

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Change the calendar-event-column layout algorithm. r=darktrojan

Change the layout algorithm to something simpler.

Previously, we did:

FOREACH event:
column = The first existing blob column 'event' would fit into.
IF column exists
Place 'event' in 'column' and as many additional neighbouring columns as possible.
ELSE
existingEvent = The first existing event that, if we shrank it to its first column, would allow 'event' to fit in the other columns.
IF existingEvent exists
Shrink 'existingEvent' to its first column.
Place 'event' in the other columns.
ELSE
Create a new column in the blob and add 'event' to it.

Now, we do (note we now use 'block' instead of 'blob'):

FOREACH event:
column = The first existing block column 'event' would fit into.
IF column exists
Place 'event' in 'column'.
ELSE
Create a new column in the block and add 'event' to it.
FOREACH event:
Grow 'event' into as many additional neighbouring columns as possible.

Basically, this delays the stretching of the event across multiple columns to a later stage. This can produce different results, but only with complex layouts with 3 or more columns. And even across these cases, there doesn't seem to be any overall advantage of one layout over the other.

Comment 5

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Convert multiday-column-top-box into an <ol> element. r=darktrojan

This simplifies the structure of the calendar-event-column, and is a step towards making the calendar views more semantic and accessible.

Depends on D129944

Comment 6

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Tidy the calendar-event-column code for events created through drag motions. r=darktrojan

Also added a FIXME explaining the race condition that the current approach relies on.

Depends on D129945

Comment 7

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Reuse calendar-event-boxes in calendar-multiday-column relayout. r=darktrojan

This saves rebuilding every element when changing a single event, or when the view is resized.

Depends on D130053

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/a81d67e6a5c4
Change the calendar-event-column layout algorithm. r=darktrojan
https://hg.mozilla.org/comm-central/rev/62b8c8481f75
Convert multiday-column-top-box into an <ol> element. r=darktrojan

Comment 9

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Use an <ol> element to list events in calendar-header-container. r=darktrojan

Depends on D130054

Comment 10

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Remove unused findColumnsForOccurrences. r=darktrojan

Depends on D130176

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/d1984c47fdb2
Tidy the calendar-event-column code for events created through drag motions. r=darktrojan
https://hg.mozilla.org/comm-central/rev/81130fad6aa5
Reuse calendar-event-boxes in calendar-multiday-column relayout. r=darktrojan
https://hg.mozilla.org/comm-central/rev/1a0dde254f65
Use an <ol> element to list events in calendar-header-container. r=darktrojan
https://hg.mozilla.org/comm-central/rev/70ab3b1c072d
Remove unused findColumnsForOccurrences. r=darktrojan

Comment 12

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Use the event target instead of mouse position for finding calendar columns. r=darktrojan

Depends on D131061

Comment 13

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Remove multiday view mStartMin and mEndMin properties. r=darktrojan

These are always 0 and 24 * 60, respectively.

Depends on D131134

Comment 14

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Simplify multiday dragging display. r=darktrojan

The previous code reuse was reduced to two cross-column methods. The cross-column methods were also de-coupled from the layout of the parent view.

Depends on D131135

Comment 15

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - For calendar multiday dragging, round the event start or end times to the nearest snapped time instead of flooring. r=darktrojan

It is more intuitive to the user that the event should snap to the 15 minute interval that is closest to the mouse.

Depends on D131061

Comment 16

[Geoff Lankow (:darktrojan)]

I've got a bunch of other dragging problems, that I don't think were caused by the current stack of patches but are worth noting somewhere:

• If you have an event that starts one day and finishes the next, and less than 24h in the view (scrolling required), dragging the end day's block around works fine, but dragging the start day's block is problematic. Looks like the distance of the scroll hasn't been accounted for.
• If you drag an item off the view (e.g. accidentally onto the status bar or into the all-day row) and back again, you're no longer moving the item around. That's an irritation we should clear up.
• TypeError: can't access property "isNegative", aDuration is null if you drag an event beyond the start of a day.

Comment 17

[Henry Wilkes (they/them) [:henry-x]]

(In reply to Geoff Lankow (:darktrojan) from comment #16)

I've got a bunch of other dragging problems, that I don't think were caused by the current stack of patches but are worth noting somewhere:

• If you have an event that starts one day and finishes the next, and less than 24h in the view (scrolling required), dragging the end day's block around works fine, but dragging the start day's block is problematic. Looks like the distance of the scroll hasn't been accounted for.

That's a good find. I get the same when dragging the event from the starting column. Something is forcing the scroll to the top (maybe the event "shadow"). The view had some bugs with scrolling whilst dragging, which is probably why it ends up acting weird. I've fixed the scroll + drag bug in a later patch. I still get the forced scroll to the top though, but at least now you can scroll back down and it looks fine again...

• If you drag an item off the view (e.g. accidentally onto the status bar or into the all-day row) and back again, you're no longer moving the item around. That's an irritation we should clear up.

Yup. This is because it is converted from a mouse event sequence to a drag event https://searchfox.org/comm-central/rev/5c142122c64106395e5f1503e36e0ef85ecf3848/calendar/base/content/calendar-multiday-view.js#1084 and its not reversed. I'm going to try and change this "move" mouse event sequence to an actual drag event.

• TypeError: can't access property "isNegative", aDuration is null if you drag an event beyond the start of a day.

See bug 1675056. I might fix this when I remove CalendarDnDContainer or when I convert the event-column "move" sequence to a drag event.

Comment 18

[Henry Wilkes (they/them) [:henry-x]]

Try run for the pending patches https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=09de2cb41a32b19323574133be7c1359a1fef8cb

Comment 19

[Henry Wilkes (they/them) [:henry-x]]

For landing, please follow the actual stack order as it is on phabricator (not the same as the order on bugzilla):

1. D131518 Bug 1713130 - For calendar multiday dragging, round the event start or end times to the nearest snapped time instead of flooring. r=darktrojan
2. D131134 Bug 1713130 - Use the event target instead of mouse position for finding calendar columns. r=darktrojan
3. D131135 Bug 1713130 - Remove multiday view mStartMin and mEndMin properties. r=darktrojan
4. D131136 Bug 1713130 - Simplify multiday dragging display. r=darktrojan

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/b8fd3d75f429
For calendar multiday dragging, round the event start or end times to the nearest snapped time instead of flooring. r=darktrojan
https://hg.mozilla.org/comm-central/rev/938a93cbea7d
Use the event target instead of mouse position for finding calendar columns. r=darktrojan
https://hg.mozilla.org/comm-central/rev/2f539bdecafc
Remove multiday view mStartMin and mEndMin properties. r=darktrojan
https://hg.mozilla.org/comm-central/rev/0da5a6d4e9a3
Simplify multiday dragging display. r=darktrojan

Comment 21

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Fix browser_eventDialogSize.js to work in isolation. r=darktrojan

The test was relying on the calendar tab being still open from the previous test.

Comment 22

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Use client positions for dragging events in multiday views. r=darktrojan

We also calculate mouse positions relative the the event column itself, rather than the parent scrollbox, which makes the event columns more self contained.

In addition, we have different behaviour when clicking an event, scrolling, and then moving the mouse. Before, the mouse would lie outside the event shadows. Now, we use the original offset that was set when clicking the event.

Depends on D132242

Comment 23

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Restructure calendar day and week views into a readable order. r=darktrojan

The UI should remain the same, with the exception of:

• Day headings are always bold (the same as in the multiweek views). Before, only 'today' was bold.
• Allday headers now have individual scrolling. Before, any overflow scrolling needed for one header would be shared by all headers.
• Allday headers now have a max height in the rotated view. Moreover, the 'rows' are no longer strictly 'equalsize', which means we save some vertical space if only one day has a long header.
• If the view is rotated, and the view overflows vertically, we can see and control both scrollbars. Before, the horizontal scrollbar would only be visible if the vertical scrollbar was at the bottom. Similarly when non-rotated.
• Using a mixture of scrolling by dragging scrollbars and using a mouse wheel is now properly supported. Before, switching from the former method to the latter would cause the view to jump to the last wheel scroll position. Moreover, scrolling with the wheel will always snap to an exact hour.
• There is no longer a 1px margin between the timebar and the event list. Instead, we have a 2px border.
• The new changes work for right-to-left. This inadvertently fixes bug 1712942, plus more.

Depends on D132243

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/acc8b454a7a3
Fix browser_eventDialogSize.js to work in isolation. r=darktrojan
https://hg.mozilla.org/comm-central/rev/0dd99f23dcee
Use client positions for dragging events in multiday views. r=darktrojan
https://hg.mozilla.org/comm-central/rev/ea72f328b8be
Restructure calendar day and week views into a readable order. r=darktrojan

Comment 25

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Use separate style measurements for multiday headings in rotated and non-rotated views. r=darktrojan

The padding and border widths of day headings differ in the rotated and non-rotated views, so we need to measure them separately, and only when the view is in the correct state.

Before this change, switching view directions could cause the headers to be offset by 1px.

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/c1b1f235bc3c
Use separate style measurements for multiday headings in rotated and non-rotated views. r=darktrojan

Comment 27

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Use hours instead of minutes for multiday calendar day start, day end, and visible time. r=darktrojan

The minute values were always multiples of 60, so we might as well use hours.

Comment 28

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Make pixels per minute more accurate for the multiday view. r=darktrojan

We no longer floor the pixels per minute to the next 0.001, which could accumulate ~1px errors over the full day.

We also use a flex display to accurately distribute this available space between the multiday hour boxes in the timebar and event columns.

Depends on D133366

Comment 29

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Move the timebar into the multiday view class. r=darktrojan

Depends on D133367

Comment 30

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Drop initial pixelsPerMinute value for the calendar multiday views. r=darktrojan

Also, use "pixelsPerMinute" rather than "mPixPerMin" to access the value.

Depends on D133368

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/690bdd42ecb2
Use hours instead of minutes for multiday calendar day start, day end, and visible time. r=darktrojan
https://hg.mozilla.org/comm-central/rev/54cb4df87e78
Make pixels per minute more accurate for the multiday view. r=darktrojan
https://hg.mozilla.org/comm-central/rev/1a2f22151357
Move the timebar into the multiday view class. r=darktrojan
https://hg.mozilla.org/comm-central/rev/89905cce78cd
Drop initial pixelsPerMinute value for the calendar multiday views. r=darktrojan

Comment 32

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Re-orient existing calendar-event-box elements on rotating the view. r=darktrojan

Otherwise, the gripbars will not move to the correct sides on rotating the view.

Comment 33

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Remove calendar-event-gripbar. r=darktrojan

A full custom element with attributes is no longer necessary.

This also fixes rotating the gripbars when rotating the view.

Comment 34

[Geoff Lankow (:darktrojan)]

There's a new failure on macOS after the most recent landings:

calendar/test/browser/views/browser_viewSwitch.js | Uncaught exception - day-view should have 9 hours visible - timed out after 50 tries

Comment 35

[Alessandro Castellani [:aleca]]

Let me know if you need help in debugging this for macOS since other platforms are not affected.

Comment 36

[Henry Wilkes (they/them) [:henry-x]]

(In reply to Geoff Lankow (:darktrojan) from comment #34)

There's a new failure on macOS after the most recent landings:

calendar/test/browser/views/browser_viewSwitch.js | Uncaught exception - day-view should have 9 hours visible - timed out after 50 tries

If this is recent, it might come from https://hg.mozilla.org/comm-central/rev/54cb4df87e78.

Looking at the screenshot (https://firefoxci.taskcluster-artifacts.net/VUdgHbmETpyfDdjcakzdbA/0/public/test_info/mozilla-test-fail-screenshot_hw5y13l4.png) there are actually 9 hours visible. So the calculation seems to be missing something https://searchfox.org/comm-central/rev/fbd21ecf0fd7cc799d1021dc1bb238edb380bcbe/calendar/test/browser/views/browser_viewSwitch.js#24-27

It may be that allowed pixel deviation (2px) is too small. But based on the screen shot, I can't see where a >=2px discrepancy could come from. Plus, locally when I set some explicit window.innerHeight values, the greatest deviation I could get was 0.125px.

(In reply to Alessandro Castellani [:aleca] from comment #35)

Let me know if you need help in debugging this for macOS since other platforms are not affected.

If you can reproduce locally, then could you confirm what the values for visiblePx and expectedPx are?

Comment 37

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Replace 'mDate' and 'mTimezone' with 'date' in multiday event header and column. r=darktrojan

Depends on D133652

Comment 38

[Geoff Lankow (:darktrojan)]

It's likely a timing issue, given that it doesn't also happen on debug builds, and sometimes doesn't happen on opt builds. I guess you just need to wait a while in the right place.

Comment 39

[Alessandro Castellani [:aleca]]

If you can reproduce locally, then could you confirm what the values for visiblePx and expectedPx are?

Unfortunately I can't reproduce it locally, the test passes every time even when running with --verify

Comment 40

[Henry Wilkes (they/them) [:henry-x]]

(In reply to Alessandro Castellani [:aleca] from comment #39)

If you can reproduce locally, then could you confirm what the values for visiblePx and expectedPx are?

Unfortunately I can't reproduce it locally, the test passes every time even when running with --verify

Nice to know that the test passes with --verify 🥲

One difference is that locally the test runs in isolation, rather than in a series. And I've had problems with this calendar test only failing in series before adding restoreCalendarViewsState, but that was for the "first visible hour", not the number of visible hours.

Can you see if this crops up when you run

./mach test comm/calendar/test/browser/views

?

Comment 41

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Create calendar-event-boxes before calculating the column layout. r=darktrojan

Depends on D133774

Comment 42

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1760318 - Add screen-reader-only headings and time-labels to the multiday views. r=darktrojan,aleca

The multiday views rely on a visual relation between the position of an event and the timebar. This patch provides this timing information for screen-readers.

Depends on D134026

Comment 43

[Alessandro Castellani [:aleca]]

Yup, I can reproduce it locally if I run the tests of the entire folder.

These are the values you asked:
visiblePx: 541
expectPx: 543.375

Comment 44

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/fa55085a3776
Remove calendar-event-gripbar. r=darktrojan
https://hg.mozilla.org/comm-central/rev/6e267ba62afa
Replace 'mDate' and 'mTimezone' with 'date' in multiday event header and column. r=darktrojan
https://hg.mozilla.org/comm-central/rev/bfdf316c148c
Create calendar-event-boxes before calculating the column layout. r=darktrojan

Comment 45

[Henry Wilkes (they/them) [:henry-x]]

(In reply to Alessandro Castellani [:aleca] from comment #43)

Yup, I can reproduce it locally if I run the tests of the entire folder.

These are the values you asked:
visiblePx: 541
expectPx: 543.375

Thanks for this. Reversing this (https://searchfox.org/comm-central/rev/fbd21ecf0fd7cc799d1021dc1bb238edb380bcbe/calendar/test/browser/views/browser_viewSwitch.js#24-26) means that you have

view.timebar.clientHeight: 1449
view.grid.scrollTopMax: 908

I can't reproduce the exact same clientHeight on my system, but I can get

view.timebar.clientHeight: 1448
view.grid.scrollTopMax: 905

and

view.timebar.clientHeight: 1451
view.grid.scrollTopMax: 907

in both the test and in normal usage of Daily.

So there seems to be a ~2px difference between the two systems, which could explain the test failure.

@aleca can you do me another favour? In the following situations:

1. In Daily, in the day view (starting at 8, showing 9 hours) and opening the Browser Toolbox, with view.timebar.clientHeight ~= 1449, where view = getCurrentView().
2. ./mach test comm/calendar/test/browser/views/browser_viewSwitch.js just after this line https://searchfox.org/comm-central/rev/fbd21ecf0fd7cc799d1021dc1bb238edb380bcbe/calendar/test/browser/views/browser_viewSwitch.js#85.
3. ./mach test comm/calendar/test/browser/views at the same line.

Give me the output of

console.log(`timebar.clientHeight: ${view.timebar.clientHeight}\ntimebar.height: ${view.timebar.getBoundingClientRect().height}\nscrollArea.top: ${view.getScrollAreaRect().top}\nscrollArea.bottom: ${view.getScrollAreaRect().bottom}\npixelsPerMinute: ${view.pixelsPerMinute}\nscrollTop: ${view.grid.scrollTop}\nscrollTopMax: ${view.grid.scrollTopMax}`);

And we can compare it with my output of

timebar.clientHeight: 1448
timebar.height: 1448
scrollArea.top: 282
scrollArea.bottom: 825
pixelsPerMinute: 1.0055555555555555
scrollTop: 483
scrollTopMax: 905

Comment 46

[Alessandro Castellani [:aleca]]

When running browser_viewSwitch.js, I get this:

timebar.clientHeight: 1447
timebar.height: 1446.6666259765625
scrollArea.top: 311.5
scrollArea.bottom: 854
pixelsPerMinute: 1.0046296296296295
scrollTop: 482
scrollTopMax: 905

When running ./mach test comm/calendar/test/browser/views, I get this:

timebar.clientHeight: 1449
timebar.height: 1449.3333740234375
scrollArea.top: 311.5
scrollArea.bottom: 854
pixelsPerMinute: 1.0064814814814815
scrollTop: 483
scrollTopMax: 908

Comment 47

[Henry Wilkes (they/them) [:henry-x]]

(In reply to Alessandro Castellani [:aleca] from comment #46)

When running browser_viewSwitch.js, I get this:

timebar.clientHeight: 1447
timebar.height: 1446.6666259765625
scrollArea.top: 311.5
scrollArea.bottom: 854
pixelsPerMinute: 1.0046296296296295
scrollTop: 482
scrollTopMax: 905

When running ./mach test comm/calendar/test/browser/views, I get this:

timebar.clientHeight: 1449
timebar.height: 1449.3333740234375
scrollArea.top: 311.5
scrollArea.bottom: 854
pixelsPerMinute: 1.0064814814814815
scrollTop: 483
scrollTopMax: 908

Thanks! So it seems in the second case that the timebar and pixelsPerMinute are set for a 1449.333px * 9hours / 24hours = 543.5px scrollable height. But the actual scrollable height in the test is 854px - 311.5px = 542.5px. So there is a 1px difference in the scrollable height between the pixelPerMinute calculation (triggered by onResize) https://searchfox.org/comm-central/rev/3fd89f9e9945ff19b9c10d62438b63d10c7eb01b/calendar/base/content/calendar-multiday-view.js#2491 and when the test is run. This doesn't happen in the first case.

It could just be a timing thing (in one of the other tests?). Also, in bug 1738689 I'll be changing the onResize call to using a ResizeObserver, so I wonder if this will be more reliable for the test.

There's also something off in both cases. I would expect

timebar.height =~ scrollTopMax + (scrollArea.bottom - scrollArea.top)

because the scrollArea is the portion of the timebar that is shown, and the rest is in scrollTopMax. But in your first case, the difference is 0.8px and the second 1.1px. This may just be an error accumulation since scrollTopMax is an integer, whilst the timebar height is floating and the scrollArea is calculated with a mix https://searchfox.org/comm-central/rev/3fd89f9e9945ff19b9c10d62438b63d10c7eb01b/calendar/base/content/calendar-multiday-view.js#3457 . Note that in the macOS examples, the scrollArea.top has a .5 value, whilst my linux example had integer values. So maybe I need to review the scrollArea calculation for when we have non-integer values.

I'm not sure what the best approach in the mean time is:

1. Change the threshold from 2px to 3px https://searchfox.org/comm-central/rev/3fd89f9e9945ff19b9c10d62438b63d10c7eb01b/calendar/test/browser/views/browser_viewSwitch.js#27
2. Disable the test on mac.

@darktrojan what do you think?

Comment 48

[Geoff Lankow (:darktrojan)]

Let's just increase the threshold. The test will still fail if there's a real problem, and that's what is important.

Comment 49

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Allow a greater pixel error bound in calendar browser_viewSwitch. r=darktrojan

Comment 50

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/66f943de1271
Allow a greater pixel error bound in calendar browser_viewSwitch. r=darktrojan

Comment 51

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Don't allow wrapping in the multiday timebar labels. r=darktrojan

Otherwise, for example when using 12 hour "en" locale, the "12:00 AM", "10:00 AM" and "10:00 PM" labels wrap in the non-rotated week view. In the rotated view, they would all wrap.

Comment 52

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/9df64eabafcd
Don't allow wrapping in the multiday timebar labels. r=darktrojan

Comment 53

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1713130 - Don't end-align multiday time labels when the view is rotated. r=darktrojan

Comment 54

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/04497f54640c
Don't end-align multiday time labels when the view is rotated. r=darktrojan

Comment 55

[Geoff Lankow (:darktrojan)]

I think we should close this since it's inactive, and do any new work in a new bug.

Comment 56

[Phabricator Automation]

Comment on attachment 9255722 [details]
Bug 1760318 - Add screen-reader-only headings and time-labels to the multiday views. r=darktrojan,aleca

Revision D134027 was moved to bug 1760318. Setting attachment 9255722 [details] to obsolete.

Comment 57

[Henry Wilkes (they/them) [:henry-x]]

(In reply to Geoff Lankow (:darktrojan) from comment #55)

I think we should close this since it's inactive, and do any new work in a new bug.

Makes sense. Only D134027 was pending, but I moved it to bug 1760318