414537 - [Task Mode] Preview pane should show link to a web page

Status: VERIFIED FIXED

(Calendar :: Tasks, defect)

Description

[Omar Bajraszewski]

At the moment when you add a link to web site to task you can't see it in the preview pane. So if you open it you have to edit the task and then click on it. The link should be visible in preview pane so a user could easy open it

Comment 1

[Lars Wohlfahrt (thetux)]

I think so, too. Would save me lots of clicks.

Comment 2

[Sven Giermann]

Attachment: Shows an attached link below description box in task preview.

Some questions regarding UI:
Is it okay to show the link below the description?
Should we add a context menu to copy the link?

I only adopted the display method of the task edit dialog...
Otherwise I accept suggestions.

Comment 3

[Sven Giermann]

Attachment: Sorry, tooltip did not work and long links did not crop...

Christian, should the tooltip display?
I realized, that it is also cropped...

Comment 4

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 301846 [details] [diff] [review]
Sorry, tooltip did not work and long links did not crop...


>+function browseItemURL(item) {
>+    var url = item.value;
>+    launchBrowser(url);
>+}
>+
Function is quite minimal, I think its ok to directly call launchBrowser(item.value) in places you would call browseItemURL. If its ok with you I'll change this before checking in.


r=philipp

Comment 5

[Sven Giermann]

(In reply to comment #4)
> Function is quite minimal, I think its ok to directly call
> launchBrowser(item.value) in places you would call browseItemURL. If its ok
> with you I'll change this before checking in.

Yes, I'm okay with that. I started to copy the function browseDocument(), but indeed it would save the whole function if you change one line:

-      onclick="browseItemURL(this)"/>
+      onclick="launchBrowser(this.value)"/>

Comment 6

[Christian Jansen]

Comment on attachment 301846 [details] [diff] [review]
Sorry, tooltip did not work and long links did not crop...

r=christian. If possible align the "Link:" label and the URL vertical. Momentarily the "Link:" label is positioned 2px above the URL. I'll + this because it is only a minor issue.

Comment 7

[Christian Jansen]

(In reply to comment #2)
> Created an attachment (id=301843) [details]
> Shows an attached link below description box in task preview.
> 
> Some questions regarding UI:
> Is it okay to show the link below the description?

place it next to the labe e.g. Link: http://www.xyz.com
> Should we add a context menu to copy the link?

makes sense.
> 
> I only adopted the display method of the task edit dialog...
This is ok for me.
> Otherwise I accept suggestions.
> 

Comment 8

[Sven Giermann]

Attachment: New version with review comments applied - leaving r+; ready for check-in

> > Is it okay to show the link below the description?
> place it next to the labe e.g. Link: http://www.xyz.com

Oh, I did this. The question was, whether the link should appear below the task description or above, as all other "headers" do; but I think it's more intuitive to read it below the description.

> > Should we add a context menu to copy the link?
> makes sense.

Because of missing locales I'm going to file a spin-off, also for the editing dialog.

Comment 9

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Sven, next time you have a patch ready for checkin, please add the checkin-needed keyword.

Checked in on both branches

-> FIXED

Comment 10

[Omar Bajraszewski]

Verified with Lt 2008020902

Comment 11

[Sven Giermann]

(In reply to comment #9)
> Sven, next time you have a patch ready for checkin, please add the
> checkin-needed keyword.

Yes, you're right - thanks for this hint!