321546 - Make view containers and item-boxes inherit from a base class

Status: RESOLVED FIXED

(Calendar :: Internal Components, defect)

Description

[Joey Minta]

Things like category class assignment and inline editing should only need to be written once and then inherited into all the different types of item-boxes.  Also, if the month-view's dayboxes and the multiday-view's columns both want to support pasting, it seems silly to try to write that twice.

Depends on displaying all day events, because I want all the different item-boxes in place before this is done.

Blocks pasting, category security stripping, and direct typing because those are things that should be inherited.

Comment 1

[gekacheka]

One previous difference is that (day/week) multiday-view item-boxes may have multiple lines of text from multiple fields, whereas month-view item-boxes are only a single field in a single line.

Also monthview item-boxes include a time.

See bug 321434 patch which separates out calendar-item-field-textbox for use in multiple fields for multiday-view.   It interacts closely with the event-box for mouse handling; that part might be put in the base item box.

Comment 2

[Joey Minta]

Attachment: core item box

Adds a base item class, and makes the month and multiday boxes inherit from it.  This means we get inline editing in the month view.  One thing this patch doesn't do is try to sort out our css story.  It's a mess.  Bug 321545 needs to do a lot of that.  This does, however, make it much easier to introduce all-day events in the multiday-view.  (I'm 90% sure that the don't even need to extend the calendar-editable-item element, they can just use it.)

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 208697 [details] [diff] [review]
core item box

When the occurrences setter is overriden in the new code, a whole bunch of the code is copy-pasted.  I think this means that it wants to be split up into several (private) methods
(eg the occurrence setter, generateTitle, buildCssCategoryString, etc.) and then these could be shared (or overridden) independently of one another.

Other than that, it looks great.

r=dmose with that fixed.

Comment 4

[Joey Minta]

Patch checked in.  Leaving the bug open for work on combining the views, but this now no longer blocks Lightning 0.1, all-day events, or stripping css chars.

Comment 5

[Joey Minta]

The more I think about it, the more it seems that pasting code belongs in the parent app, not in the particular views.  As such, I don't really see anything significant that the views could share.  Marking fixed, but if someone has any other suggestions for how the views can benefit from inheriting from a base class, please feel free to reopen.