Details
Diff Detail
- Repository
- rCOMMCENTRAL comm-central
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| calendar/base/content/dialogs/calendar-event-dialog-recurrence.xhtml | ||
|---|---|---|
| 24 | The width and height are not being persisted. I'm not sure why this worked before and not now. | |
On closer inspection, this dialog had a lot of problems on trunk as well (esp. with regards to making it smaller)
Thanks for pointing out the resize problem. Unfortunately, even with the changes, the layout calculations do not work exactly because they do not take the window padding and exact sizes into account. It is also slow to change.
We could simplify it using a grid display on #recurrence-preview that only occupies the available size and has grid-auto-columns: min-content. On resizing, we measure its size and calculate how many minimonth elements would fit. Then you just need to change the number of minimonth elements and set the style.gridRow of each minimonth element to the corresponding value.
| calendar/base/content/dialogs/calendar-event-dialog-recurrence.js | ||
|---|---|---|
| 64–65 | We should use the #recurrence-preview content width, which takes padding etc into account. | |
| 64–65 | This can be cached to a property because the size isn't expected to change. | |
| 64–65 | If you use a grid layout, then this can be reduced to appending or removing excess minimonth elements. | |
| 65 | We should use the #recurrence-preview content height. | |
| 475 | This needs to be called after the "load" event. If it is called during "DOMContentLoaded" it throws errors. | |
| calendar/base/content/dialogs/calendar-event-dialog-recurrence.xhtml | ||
| 771–772 | Remove these extra two. | |
| 771–772 | This could be a single <div id="recurrence-preview"> with a grid layout. | |
| calendar/base/content/dialogs/calendar-event-dialog-recurrence.js | ||
|---|---|---|
| 64–65 | That is what was used (#recurrence-preview is this.node). | |
| 64–65 | There is much that could potentially be improved here, but I'd like to just do what I came for. | |
The current patch still results in clipping at certain sizes. If you don't want to use a CSS grid and want to keep it similar to how it is now, you can use the suggested edits.
| calendar/base/content/dialogs/calendar-event-dialog-recurrence.js | ||
|---|---|---|
| 31 | Suggestion for initial values. | |
| 64–65 | This is a suggestion for more accurate measurements. And to also prevent rebuilding on every resize, which makes it much smoother. | |
| 77 | Remove this. | |
| calendar/base/content/dialogs/calendar-event-dialog-recurrence.xhtml | ||
| 771–772 | Remove the extra months. | |
| 771–772 | Allow the box to re-adjust to any size. | |
| 771–772 | Make sure there is enough room for one minimonth. | |
| calendar/base/content/dialogs/calendar-event-dialog-recurrence.js | ||
|---|---|---|
| 64–65 | The 4 extra px doesn't work for me, it will make the initial window just have 2 months. Without the 4 extra px things look very good now it seems. Thanks for the suggestions! | |
Looks good. Just a stray comment.
| calendar/base/content/dialogs/calendar-event-dialog-recurrence.js | ||
|---|---|---|
| 64–65 | Remove this comment since the patch didn't do this. | |
Working on other platforms but there's test failures for Windows:
comm/calendar/test/browser/recurrence/browser_lastDayOfMonth.js
comm/calendar/test/browser/recurrence/browser_weeklyN.js
comm/calendar/test/browser/recurrence/browser_lastDayOfMonth.js