Closed
Bug 878604
Opened 11 years ago
Closed 11 years ago
Convert search dialog folderpicker to newer binding, it's prettier -- and rdf free.
Categories
(Thunderbird :: Search, defect)
Thunderbird
Search
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 24.0
People
(Reporter: alta88, Assigned: alta88)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 3 obsolete files)
10.93 KB,
patch
|
alta88
:
review+
alta88
:
ui-review+
|
Details | Diff | Splinter Review |
No description provided.
Seems like that picker was forgotten along the way..
Assignee: nobody → alta88
Attachment #757158 -
Flags: ui-review?(richard.marti)
Attachment #757158 -
Flags: review?(mkmelin+mozilla)
fix SearchDialog.js - get rid of folder to URI to folder to URI hackiness, remove an unused function, ensure the new folderpicker icon is correct for css. the Filter dialog picker should be likewise fixed.
Attachment #757158 -
Attachment is obsolete: true
Attachment #757158 -
Flags: ui-review?(richard.marti)
Attachment #757158 -
Flags: review?(mkmelin+mozilla)
Attachment #757179 -
Flags: ui-review?(richard.marti)
Attachment #757179 -
Flags: review?(mkmelin+mozilla)
Nice. But not sure what you mean with the Filter dialog. That one was already converted.
I just meant the part about the icon missing and causing the box to be shrunken; the menulist just needs a class="folderMenuItem" and making sure _setCssSelectors is in there. And the Run Filters picker shouldn't show/select the account, as it doesn't apply (bug 264384). Unless of course, Run Filters gets a Subfolders option (like search).
Thanks, didn't notice the folderMenuItem class is also used on the menulists. I was mostly cleaning up the class="menulist-menupopup" on the menupopup child. So if you want to cleanup the icons on the menulist, then also the New Folder dialog is missing it. Or maybe they are intentionally not there in some places. Anyway, file the bug please (whether you intend to work on it or not) and CC me there.
Status: NEW → ASSIGNED
Comment 6•11 years ago
|
||
Comment on attachment 757179 [details] [diff] [review] patch This looks good. This makes the folder picker on main toolbar (#locationFolders), the two menulists in filter dialog, the picker in account manager and now in search dialog consistent without a tree. The only difference I saw was the folder name instead the "choose this folder" menuitem, but I saw this is addressed in bug 878805.
Attachment #757179 -
Flags: ui-review?(richard.marti) → ui-review+
this update just adds the IsFeedFolder="true" attribute to menulists using _setCssProperties. once this is in, and bug 878805 is verified, the folderMenus.css can be updated to show both subs/no subs feed folder icons correctly.
Attachment #757179 -
Attachment is obsolete: true
Attachment #757179 -
Flags: review?(mkmelin+mozilla)
Attachment #757435 -
Flags: ui-review+
Attachment #757435 -
Flags: review?(mkmelin+mozilla)
(In reply to alta88 from comment #7) > this update just adds the IsFeedFolder="true" attribute to menulists using > _setCssProperties. once this is in, and bug 878805 is verified, the > folderMenus.css can be updated to show both subs/no subs feed folder icons > correctly. I don't think bug 878805 will add any new css. So if you need some, please add it. (In reply to Richard Marti [:Paenglab] from comment #6) > The only difference I saw was the folder name instead the "choose this > folder" menuitem, but I saw this is addressed in bug 878805. Actually bug 878805 proposes to make this change ("file here" -> folder name) also in all the other pickers. So depends on which direction you want this to be addressed.
Comment 9•11 years ago
|
||
(In reply to :aceman from comment #8) > Actually bug 878805 proposes to make this change ("file here" -> folder > name) also in all the other pickers. So depends on which direction you want > this to be addressed. I showed only the difference. The direction "file here" -> folder name would for me make sense. At the beginning of my TB usage I asked me sometimes, which folder is "choose this folder". This change could help not so experienced users.
Assignee | ||
Comment 10•11 years ago
|
||
(In reply to :aceman from comment #8) > (In reply to alta88 from comment #7) > > this update just adds the IsFeedFolder="true" attribute to menulists using > > _setCssProperties. once this is in, and bug 878805 is verified, the > > folderMenus.css can be updated to show both subs/no subs feed folder icons > > correctly. > I don't think bug 878805 will add any new css. So if you need some, please > add it. > This was just a note that an issue in bug 539837 can be completed post bug 878805, which I will note there.. (In reply to Richard Marti [:Paenglab] from comment #9) > (In reply to :aceman from comment #8) > > Actually bug 878805 proposes to make this change ("file here" -> folder > > name) also in all the other pickers. So depends on which direction you want > > this to be addressed. > > I showed only the difference. The direction "file here" -> folder name would > for me make sense. At the beginning of my TB usage I asked me sometimes, > which folder is "choose this folder". This change could help not so > experienced users. I'm still confused by it!
Comment 11•11 years ago
|
||
Comment on attachment 757435 [details] [diff] [review] patch Review of attachment 757435 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, it looks like you can also get rid of the mail/ css for folderTree and folderTreeChildren - <http://mxr.mozilla.org/comm-central/search?string=foldersTree&filter=^[^\0]*$> since that's the last binding using them.
Attachment #757435 -
Flags: review?(mkmelin+mozilla) → review+
Assignee | ||
Comment 12•11 years ago
|
||
remove unnecessary css per comment.
Attachment #757435 -
Attachment is obsolete: true
Attachment #758784 -
Flags: ui-review+
Attachment #758784 -
Flags: review+
Keywords: checkin-needed
Comment 13•11 years ago
|
||
https://hg.mozilla.org/comm-central/rev/d031447ec78b
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 24.0
Updated•10 years ago
|
Blocks: mail-killrdf
You need to log in
before you can comment on or make changes to this bug.
Description
•