Closed
Bug 425998
Opened 17 years ago
Closed 17 years ago
History items (Today, Yesterday, etc.) should use a calendar icon
Categories
(Firefox :: Theme, defect)
Firefox
Theme
Tracking
()
VERIFIED
FIXED
Firefox 3
People
(Reporter: faaborg, Assigned: mcdavis941.bugs)
References
Details
Attachments
(3 files, 4 obsolete files)
614 bytes,
image/png
|
Details | |
637 bytes,
image/png
|
Details | |
4.78 KB,
patch
|
Gavin
:
review+
beltzner
:
ui-review+
beltzner
:
approval1.9+
|
Details | Diff | Splinter Review |
The top level items in the history sidebar "Today, Yesterday, 2 Days Ago, 5 Days ago, Older than 6 days ago" should use the icon /places/calendar.png
This will be landing for windows xp and vista soon. Linux and OS X should use a similar image in the same location of pinstripe and gnomestripe.
Comment 1•17 years ago
|
||
Linux now has this icon landed and ready for use in gnomestripe, as per bug 418868, comment #66.
Reporter | ||
Comment 2•17 years ago
|
||
Requesting blocking since we have the icons.
Flags: blocking-firefox3?
Comment 3•17 years ago
|
||
OSX has a clock icon already - do we know where it is?
Not blocking, but an obvious easy-to-take if someone can post a CSS fix that points to the new icon.
Flags: wanted-firefox3+
Flags: blocking-firefox3?
Flags: blocking-firefox3-
Comment 4•17 years ago
|
||
The CSS fix, tested on Firefox3b4:
#historyTree treechildren::-moz-tree-image(title, folder) {
list-style-image: url("chrome://browser/skin/places/calendar.png") !important;
-moz-image-region: rect(0px, 16px, 16px, 0px) !important;
}
Reporter | ||
Comment 5•17 years ago
|
||
>OSX has a clock icon already - do we know where it is?
The clock is unrelated to this change.
Assignee | ||
Comment 6•17 years ago
|
||
I can turn Lim Chee Aun's code into a patch. Alex, could I see the icons, so I know what states (hover, open, etc.) to code for?
Status: NEW → ASSIGNED
Assignee | ||
Updated•17 years ago
|
Assignee: nobody → mcdavis941.bugs
Status: ASSIGNED → NEW
Comment 7•17 years ago
|
||
(In reply to comment #6)
> ... so I know what states (hover, open, etc.) to code for?
Wow, even a calendar icon have states? Is it necessary?
Assignee | ||
Comment 8•17 years ago
|
||
>(In reply to comment #6)
> Wow, even a calendar icon have states? Is it necessary?
Good question, I don't know. Probably not hover, since nothing else does that I can see, but maybe open because it's a folder.
I didn't ask ... do you want to take this bug? I assumed you would already have if you wanted to, but maybe that was my mistake.
Comment 9•17 years ago
|
||
(In reply to comment #8)
> >(In reply to comment #6)
> > Wow, even a calendar icon have states? Is it necessary?
>
> Good question, I don't know. Probably not hover, since nothing else does that
> I can see, but maybe open because it's a folder.
"Open a calendar" icon? But well, I'm not really a fan of open/closed folder icons anyway since I think they're useless and there are already open/closed twisties. Depends on devs then.
> I didn't ask ... do you want to take this bug?
Feel free to take it. I don't know how to write a patch though.
Assignee | ||
Updated•17 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 10•17 years ago
|
||
Original code by Lim Chee Aun
Attachment #313031 -
Flags: review?(gavin.sharp)
Comment 11•17 years ago
|
||
Mind including gnomestripe in your patch, too?
browser/themes/gnomestripe/browser/places/places.css
#historyTree treechildren::-moz-tree-image(title, folder) {
list-style-image: url("chrome://browser/skin/places/calendar.png");
}
Assignee | ||
Comment 12•17 years ago
|
||
(In reply to comment #11)
> Mind including gnomestripe in your patch, too?
Right.
Reporter | ||
Comment 13•17 years ago
|
||
Example of the luna calendar (single state)
Reporter | ||
Comment 14•17 years ago
|
||
Example of the aero calendar (single state)
Assignee | ||
Comment 15•17 years ago
|
||
For gnomestripe, pinstripe, and winstripe.
Reed: confirmed that -moz-image-region: auto was required on winstripe.
Attachment #313031 -
Attachment is obsolete: true
Attachment #313044 -
Flags: review?(gavin.sharp)
Attachment #313031 -
Flags: review?(gavin.sharp)
Comment 16•17 years ago
|
||
(In reply to comment #15)
> Reed: confirmed that -moz-image-region: auto was required on winstripe.
No, I was just asking you if it was required or not... Did your testing show that it was? If it isn't, the line can be dropped, but if it is, we need it. :)
Comment 17•17 years ago
|
||
oh, you're answering me! n/m... *sigh*
Comment 18•17 years ago
|
||
Correct me if I'm wrong, I'm just looking at the code, it seems you apply a calendar icon to every folder? What if the user changes it to by Site or Date & Site?
Comment 19•17 years ago
|
||
(In reply to comment #18)
> Correct me if I'm wrong, I'm just looking at the code, it seems you apply a
> calendar icon to every folder? What if the user changes it to by Site or Date &
> Site?
Ugh, you're right. The code applies the calendar icon to every folder! In that case, the code needs to be revised and I'm not sure if something like '
treechildren::-moz-tree-image(title, calendar)' exists or not.
Comment 20•17 years ago
|
||
imo that should be fixed in places frontend code providing an attribute for history containers like has been done for tag containers (they can be recognized using PlacesUtils.nodeIsDay() or checking parent result type).
Assignee | ||
Comment 21•17 years ago
|
||
(In reply to comment #20)
> imo that should be fixed in places frontend code providing an attribute for
> history containers like has been done for tag containers (they can be
> recognized using PlacesUtils.nodeIsDay() or checking parent result type).
>
Yeah, there needs to be a new property to distinguish the two cases. Checking all the existing properties (that I could find), there isn't any that was found on a folder representing a date (meaning it should have a calendar icon) that wasn't also on a folder representing a site (with pages beneath it) in the history sidebar.
I'd vote for something like 'dateFolder'.
Assignee | ||
Updated•17 years ago
|
Attachment #313044 -
Attachment is obsolete: true
Attachment #313044 -
Flags: review?(gavin.sharp)
Comment 22•17 years ago
|
||
everything should be ready now
Comment 23•17 years ago
|
||
mcdavis: so, if everything's ready, can we get that patch?
Assignee | ||
Comment 24•17 years ago
|
||
Sure, today.
Assignee | ||
Comment 25•17 years ago
|
||
Where should I be looking for the calendar icon for Pinstripe? It looks like it hasn't landed yet, is that right?
I see Alex's comment about it being in the same location for OSX and Linux, I'm just double-checking.
Assignee | ||
Comment 26•17 years ago
|
||
Leaves off OS X changes in favor of Bug 430693
Comment 27•17 years ago
|
||
IIRC there is another reference to day containers maybe in browser.css?
Assignee | ||
Comment 28•17 years ago
|
||
(In reply to comment #27)
> IIRC there is another reference to day containers maybe in browser.css?
>
That's true. Does that case occur in the UI? I saw the rule, but do you ever have a dayContainer as a bookmark item?
Comment 29•17 years ago
|
||
you could create a saved search with those in, maybe in the toolbar or in the menu
Assignee | ||
Comment 30•17 years ago
|
||
(In reply to comment #29)
> you could create a saved search with those in, maybe in the toolbar or in the
> menu
>
thanks. i'll update to handle this case.
Assignee | ||
Comment 31•17 years ago
|
||
Attachment #317569 -
Attachment is obsolete: true
Comment 32•17 years ago
|
||
treechildren::-moz-tree-image(title, query, dayContainer) {
list-style-image: url("moz-icon://stock/gtk-directory?size=menu");
}
+/* calendar icon for folders grouping items by date */
+treechildren::-moz-tree-image(title, query, dayContainer) {
+ list-style-image: url("chrome://browser/skin/places/calendar.png");
+}
+
this appear like a dupe(?)
Assignee | ||
Updated•17 years ago
|
Attachment #317631 -
Flags: ui-review?(beltzner)
Attachment #317631 -
Flags: review?(gavin.sharp)
Updated•17 years ago
|
Attachment #317631 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Updated•17 years ago
|
Keywords: checkin-needed
Assignee | ||
Updated•17 years ago
|
Keywords: checkin-needed
Updated•17 years ago
|
Attachment #317631 -
Flags: approval1.9?
Comment 34•17 years ago
|
||
Comment on attachment 317631 [details] [diff] [review]
browser.css and places.css v5
re-request approval once ui review has been completed.
Attachment #317631 -
Flags: approval1.9?
Comment 35•17 years ago
|
||
Comment on attachment 317631 [details] [diff] [review]
browser.css and places.css v5
uir+a=beltzner
Attachment #317631 -
Flags: ui-review?(beltzner)
Attachment #317631 -
Flags: ui-review+
Attachment #317631 -
Flags: approval1.9+
Updated•17 years ago
|
Keywords: checkin-needed
Comment 36•17 years ago
|
||
mozilla/browser/themes/gnomestripe/browser/browser.css 1.215
mozilla/browser/themes/gnomestripe/browser/places/places.css 1.33
mozilla/browser/themes/winstripe/browser/browser.css 1.216
mozilla/browser/themes/winstripe/browser/places/places.css 1.25
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3
Comment 37•17 years ago
|
||
Verified with
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008050506 Minefield/3.0pre ID:2008050506
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9pre) Gecko/2008050504 Minefield/3.0pre
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•