Closed
Bug 414537
Opened 17 years ago
Closed 17 years ago
[Task Mode] Preview pane should show link to a web page
Categories
(Calendar :: Tasks, defect)
Calendar
Tasks
Tracking
(Not tracked)
VERIFIED
FIXED
0.8
People
(Reporter: omar.bajraszewski, Assigned: giermann)
Details
Attachments
(1 file, 2 obsolete files)
6.39 KB,
patch
|
giermann
:
review+
giermann
:
ui-review+
|
Details | Diff | Splinter Review |
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•17 years ago
|
||
I think so, too. Would save me lots of clicks.
Assignee | ||
Comment 2•17 years ago
|
||
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.
Attachment #301843 -
Flags: ui-review?(christian.jansen)
Attachment #301843 -
Flags: review?(philipp)
Assignee | ||
Comment 3•17 years ago
|
||
Christian, should the tooltip display?
I realized, that it is also cropped...
Attachment #301843 -
Attachment is obsolete: true
Attachment #301846 -
Flags: ui-review?(christian.jansen)
Attachment #301846 -
Flags: review?(philipp)
Attachment #301843 -
Flags: ui-review?(christian.jansen)
Attachment #301843 -
Flags: review?(philipp)
Updated•17 years ago
|
Assignee: nobody → giermann
OS: Windows XP → All
Hardware: PC → All
Version: Mozilla 1.8 Branch → unspecified
Updated•17 years ago
|
Status: NEW → ASSIGNED
Comment 4•17 years ago
|
||
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
Attachment #301846 -
Flags: review?(philipp) → review+
Assignee | ||
Comment 5•17 years ago
|
||
(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•17 years ago
|
||
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.
Attachment #301846 -
Flags: ui-review?(christian.jansen) → ui-review+
Comment 7•17 years ago
|
||
(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.
>
Assignee | ||
Comment 8•17 years ago
|
||
> > 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.
Attachment #301846 -
Attachment is obsolete: true
Attachment #302095 -
Flags: ui-review+
Attachment #302095 -
Flags: review+
Comment 9•17 years ago
|
||
Sven, next time you have a patch ready for checkin, please add the checkin-needed keyword.
Checked in on both branches
-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Target Milestone: --- → 0.8
Assignee | ||
Comment 11•17 years ago
|
||
(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!
You need to log in
before you can comment on or make changes to this bug.
Description
•