Integration of react equivalent of calendar print dialog

NEW
Unassigned

Status

2 years ago
8 months ago

People

(Reporter: arshad, Unassigned, Mentored)

Tracking

Details

Attachments

(1 attachment, 8 obsolete attachments)

(Reporter)

Description

2 years ago
Posted patch print-dialog-final.patch (obsolete) β€” β€” Splinter Review
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20170824100243
(Reporter)

Updated

2 years ago
Mentor: philipp
(Reporter)

Comment 1

2 years ago
Posted patch print-dialog-final.patch (obsolete) β€” β€” Splinter Review
Removed extraneous files.
Attachment #8901057 - Attachment is obsolete: true
Attachment #8901058 - Flags: feedback?(richard.marti)
Attachment #8901058 - Flags: feedback?(philipp)
(Reporter)

Comment 2

2 years ago
Posted patch print-dialog-final.patch (obsolete) β€” β€” Splinter Review
Attachment #8901058 - Attachment is obsolete: true
Attachment #8901058 - Flags: feedback?(richard.marti)
Attachment #8901058 - Flags: feedback?(philipp)
Attachment #8901063 - Flags: feedback?(richard.marti)
Attachment #8901063 - Flags: feedback?(philipp)
(Reporter)

Comment 3

2 years ago
Posted patch print-dialog-final.patch (obsolete) β€” β€” Splinter Review
Attachment #8901063 - Attachment is obsolete: true
Attachment #8901063 - Flags: feedback?(richard.marti)
Attachment #8901063 - Flags: feedback?(philipp)
Attachment #8901082 - Flags: feedback?(richard.marti)
Attachment #8901082 - Flags: feedback?(philipp)
Comment on attachment 8901082 [details] [diff] [review]
print-dialog-final.patch

Thank you for the hard work. It looks promising but it has some issues.

First, I never see anything in the preview area except the white background.

Then the date fields don't show a datepicker. maybe it's an issue that the toolkit datetimepicker isn't initialized correctly in TB as a timepicker in a test mail also shows no picker like Firefox.

On opening the dialog I see this error: ReferenceError: assignment to undeclared variable eventWithDueDate[Learn More]  calendar-print-dialog.js:293:11

I know, you want to do this later but for the record: The dialog has no persistence of size and position.
Attachment #8901082 - Flags: feedback?(richard.marti)
(Reporter)

Comment 5

2 years ago
Posted patch print-dialog-final.patch (obsolete) β€” β€” Splinter Review
Changes:

- Iframe now gets the html content based upon selections
- Removed the undeclared variable warning

Datepicker can be enabled by changing dom.forms.datetime.. variables. Although there is some issue with datepicker in nightly, it automaticlly closes up. Any other solutions for datepicker?
Attachment #8901082 - Attachment is obsolete: true
Attachment #8901082 - Flags: feedback?(philipp)
Attachment #8901413 - Flags: feedback?(richard.marti)
Attachment #8901413 - Flags: feedback?(philipp)
(Reporter)

Comment 6

2 years ago
(In reply to Richard Marti (:Paenglab) from comment #4)
> Comment on attachment 8901082 [details] [diff] [review]
> print-dialog-final.patch
> 
> Thank you for the hard work. It looks promising but it has some issues.
> 
> First, I never see anything in the preview area except the white background.
> 
> Then the date fields don't show a datepicker. maybe it's an issue that the
> toolkit datetimepicker isn't initialized correctly in TB as a timepicker in
> a test mail also shows no picker like Firefox.
> 
> On opening the dialog I see this error: ReferenceError: assignment to
> undeclared variable eventWithDueDate[Learn More] 
> calendar-print-dialog.js:293:11
> 
> I know, you want to do this later but for the record: The dialog has no
> persistence of size and position.

Can't do much with datepicker :( 

Could you please test this patch with date range and check whether the dialog is working correctly or not?
Comment on attachment 8901413 [details] [diff] [review]
print-dialog-final.patch

Looks good, now also with preview in the dialog. I couldn't made the datepicker work also with all dom.forms.datetime enabled.

I haven't tested if it prints, but I'm sure did it. :)
Attachment #8901413 - Flags: feedback?(richard.marti) → feedback+
(Reporter)

Comment 8

2 years ago
Posted patch print-dialog-final.patch (obsolete) β€” β€” Splinter Review
Changes made in this patch -

- Datepicker now catches all the events and tasks
- initial date is now automatcally set
Attachment #8901413 - Attachment is obsolete: true
Attachment #8901413 - Flags: feedback?(philipp)
Attachment #8901556 - Flags: feedback?(philipp)
(Reporter)

Comment 9

2 years ago
Posted patch print-dialog-final.patch (obsolete) β€” β€” Splinter Review
Changes made in this patch - 

- printEvents and printTasks checkbox are now persistent
- tab size corrected for css/html files
- persistent screenX, screenY
- persistent height and width of dialog
- fixes bug 15232
Attachment #8901556 - Attachment is obsolete: true
Attachment #8901556 - Flags: feedback?(philipp)
Attachment #8902258 - Flags: feedback?(richard.marti)
Attachment #8902258 - Flags: feedback?(philipp)
Comment on attachment 8902258 [details] [diff] [review]
print-dialog-final.patch

Persistence works, thanks.
Attachment #8902258 - Flags: feedback?(richard.marti) → feedback+
(Reporter)

Comment 11

2 years ago
Posted patch cal-print-dialog.patch (obsolete) β€” β€” Splinter Review
Removed some comments from store.js file in print dialog folder and added license comment in react-utils.js
Attachment #8902258 - Attachment is obsolete: true
Attachment #8902258 - Flags: feedback?(philipp)
Attachment #8917886 - Flags: review?(richard.marti)
Attachment #8917886 - Flags: review?(philipp)
Comment on attachment 8917886 [details] [diff] [review]
cal-print-dialog.patch

Removing my review until the common files are in a separate patch.

Please check also that both bugs work together. You can also define which bug/patch needs to be applied first if they change the same files.
Attachment #8917886 - Flags: review?(richard.marti)
Component: Untriaged → General
Product: Thunderbird → Calendar
Version: 57 Branch → unspecified
(Reporter)

Comment 13

a year ago
Posted patch print-dialog.patch β€” β€” Splinter Review
This patch is an old patch uploaded before the latest patch which fails on applying. Common files aren't separated into separate patch as doing that at this stage is very difficult task.
Attachment #8917886 - Attachment is obsolete: true
Attachment #8917886 - Flags: review?(philipp)
Status: UNCONFIRMED → NEW
Ever confirmed: true
You need to log in before you can comment on or make changes to this bug.