import/export functionality for lightning

RESOLVED FIXED in Lightning 0.1

Status

Calendar
Lightning Only
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: dmose, Assigned: Joey Minta)

Tracking

Trunk
Lightning 0.1
Dependency tree / graph

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

12 years ago
Lightning needs to be able import and export both events and calendars.
(Assignee)

Comment 1

12 years ago
import-export code currently requires undo-redo.  Adding a blocker for now.  dmose, we can either work to land undo/redo or add some small if/else blocks into the import-export code.  Opinions?
Depends on: 293766
(Reporter)

Comment 2

12 years ago
I'd vote for just landing undo/redo, though I don't know that either that (or this) needs to block 0.1 from shipping.
(Reporter)

Comment 3

12 years ago
After discusssion with Joey, we've decided that this is too important to ship 0.1 without.  Furthermore, getting undo/redo right looks a bunch of work, so adding a few startBatchTransaction/endBatchTransaction stub functions sounds like a cost worth temporarily bearing.
Blocks: 298366
No longer depends on: 293766
Target Milestone: Lightning 0.2 → Lightning 0.1

Updated

12 years ago
Blocks: 323183
(Assignee)

Comment 4

12 years ago
Created attachment 208479 [details] [diff] [review]
import patch v1

Full patch to add import to Lightning.  Given that we currently only try to export selected events and that only one event can be selected at a time in Lightning, I'm inclined to wait on wiring up export.  Publish is a usable workaround.
Assignee: nobody → jminta
Status: NEW → ASSIGNED
(Assignee)

Comment 5

12 years ago
Created attachment 208480 [details] [diff] [review]
import patch v1 -w

patch -w

Adds import functionality to Lightning.  getSelectedCalendar() doesn't exist in Lightning, so I went ahead and wired up a quick calendarPicker dialog.  It should be flexible enough to be used for other purposes as well.  I also had to rename getStringBundle because there was a name conflict in Thunderbird.
Attachment #208480 - Flags: first-review?(mvl)

Updated

12 years ago
No longer blocks: 323183
Comment on attachment 208480 [details] [diff] [review]
import patch v1 -w

Why do we need this calendar-prompt, when you have ltnSelectedCalendar() ? Won't that work, and make the patch much shorter?
(Assignee)

Comment 7

12 years ago
(In reply to comment #6)
> (From update of attachment 208480 [details] [diff] [review] [edit])
> Why do we need this calendar-prompt, when you have ltnSelectedCalendar() ?
> Won't that work, and make the patch much shorter?
> 
Yes, that is an option, but rather than creating an if/else block for sunbird/lightning uses, this seemed like an appropriate time to remove the "// XXX Ask for a calendar to import into" comment and do things correctly.
Do we really want to bother the user with selecting a calendar? There already is a way to select a calendar...
(Reporter)

Comment 9

12 years ago
Part of the problem here is that this context menu is arguably in the wrong place.  Its on the entire calendar pane rather than just the individual items.  At some point we'll have to take a look at this from a UI design standpoint and decide what the best factoring really is.  For now, my inclination is that making the user go through a dialog when that could be avoided seems something to avoid just as far as efficiency of use goes.
(Assignee)

Comment 10

12 years ago
(In reply to comment #9)
> Part of the problem here is that this context menu is arguably in the wrong
> place.  Its on the entire calendar pane rather than just the individual items. 

Lightning doesn't have it in a context menu.  I added it to the 'Calendar' menu.  We're going to need to do a least a little UI planning here.  Suggestions? 

(Reporter)

Comment 11

12 years ago
I vote we ask the UI guys.  :-)
(Reporter)

Comment 12

12 years ago
OK, just talked to mconnor about this, and what came out of that discussion was the following proposal: long-term, we may want something wizard-like for both publish and export since they share a lot of functionality.  For 0.1, having the drop-down menu put up a calendar picker, as well as having an item in the context menu that just exports the selected one should be sufficient.

Comment 13

12 years ago
CC'ing myself on this one. (We need the ability to vote for more than 3 calendar bugs.)
(Assignee)

Comment 14

12 years ago
Created attachment 211382 [details] [diff] [review]
import patch v2

There's a lot here. :-/  This adds import-export support for lightning.  In line with comment #12, we add both toolbar-menu items, that give the user a list of calendars to choose from (if there is more than 1 calendar) and context menu items, that go directly into that calendar.  It also introduces these context menu items into Sunbird.
Attachment #208479 - Attachment is obsolete: true
Attachment #208480 - Attachment is obsolete: true
Attachment #211382 - Flags: first-review?(mvl)
Attachment #208480 - Flags: first-review?(mvl)
(Assignee)

Updated

12 years ago
Blocks: 325475
Comment on attachment 211382 [details] [diff] [review]
import patch v2

>Index: calendar/base/content/calendarPicker.xul

I'm not too happy with this name. All the other *Picker files I could think of are xbl widgets, while this is a window. Can you rename it?

>+<!ENTITY calendar.import.into.label              "Import into..." >
>+<!ENTITY calendar.export.from.label              "Export from..." >
>+<!ENTITY calendar.import.calendar                "Import..." >
>+<!ENTITY calendar.export.calendar                "Export Calendar..." >

iirc, we use real ellipsis characters, not just three dots.

>+  <menuitem label="&calendar.import.into.label;"
>+            id="calpopup-import"
>+            oncommand="loadEventsFromFile(getSelectedCalendarOrNull())"/>

In sunbird, i right clicked on a calendar in the calendars tab, and the menu had 'Export from..." That wording sounded weird to me. I think "Export Calendar" would be a better choice. I already selected a calendar by right-clicking on it.

Rest of the patch looks good.
(Assignee)

Comment 16

12 years ago
Created attachment 211750 [details] [diff] [review]
import patch v3

Patch updated to previous review comments.
Attachment #211382 - Attachment is obsolete: true
Attachment #211750 - Flags: first-review?(mvl)
Attachment #211382 - Flags: first-review?(mvl)
Comment on attachment 211750 [details] [diff] [review]
import patch v3

>Index: calendar/base/content/chooseCalendarDialog.xul
>+    <vbox id="dialog-box">
>+        <textbox id="prompt" class="plain" readonly="true" multiline="true" rows="2"/>

I think that should be a label, not a textbox. Textboxes are generally for editing, labels for, uhm, lables, just like how you use it here.
If wrapping is the problem, don't set the value, but add a checld textnode.

r=mvl with that fixed
Attachment #211750 - Flags: first-review?(mvl) → first-review+
(Assignee)

Comment 18

12 years ago
Patch checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.