Last Comment Bug 878604 - Convert search dialog folderpicker to newer binding, it's prettier -- and rdf free.
: Convert search dialog folderpicker to newer binding, it's prettier -- and rdf...
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Search (show other bugs)
: unspecified
: All All
: -- normal (vote)
: Thunderbird 24.0
Assigned To: alta88
:
Mentors:
: 442802 886912 (view as bug list)
Depends on:
Blocks: mail-killrdf 507669
  Show dependency treegraph
 
Reported: 2013-06-02 12:09 PDT by alta88
Modified: 2014-04-24 18:25 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (6.21 KB, patch)
2013-06-02 12:12 PDT, alta88
no flags Details | Diff | Review
patch (8.92 KB, patch)
2013-06-02 14:08 PDT, alta88
richard.marti: ui‑review+
Details | Diff | Review
patch (9.67 KB, patch)
2013-06-03 09:09 PDT, alta88
mkmelin+mozilla: review+
alta88: ui‑review+
Details | Diff | Review
patch for checkin (10.93 KB, patch)
2013-06-05 13:55 PDT, alta88
alta88: review+
alta88: ui‑review+
Details | Diff | Review

Description alta88 2013-06-02 12:09:08 PDT

    
Comment 1 alta88 2013-06-02 12:12:10 PDT
Created attachment 757158 [details] [diff] [review]
patch


Seems like that picker was forgotten along the way..
Comment 2 alta88 2013-06-02 14:08:39 PDT
Created attachment 757179 [details] [diff] [review]
patch


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.
Comment 3 :aceman 2013-06-03 03:55:54 PDT
Nice.
But not sure what you mean with the Filter dialog. That one was already converted.
Comment 4 alta88 2013-06-03 06:19:02 PDT
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).
Comment 5 :aceman 2013-06-03 06:30:27 PDT
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.
Comment 6 Richard Marti (:Paenglab) 2013-06-03 08:56:46 PDT
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.
Comment 7 alta88 2013-06-03 09:09:08 PDT
Created attachment 757435 [details] [diff] [review]
patch


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.
Comment 8 :aceman 2013-06-03 09:35:08 PDT
(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 Richard Marti (:Paenglab) 2013-06-03 09:45:47 PDT
(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.
Comment 10 alta88 2013-06-03 09:57:01 PDT
(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 Magnus Melin 2013-06-05 13:33:28 PDT
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.
Comment 12 alta88 2013-06-05 13:55:07 PDT
Created attachment 758784 [details] [diff] [review]
patch for checkin


remove unnecessary css per comment.
Comment 13 Ryan VanderMeulen [:RyanVM] 2013-06-05 14:54:08 PDT
https://hg.mozilla.org/comm-central/rev/d031447ec78b
Comment 14 :aceman 2013-06-27 06:10:48 PDT
*** Bug 886912 has been marked as a duplicate of this bug. ***
Comment 15 Joshua Cranmer [:jcranmer] 2013-10-10 08:01:43 PDT
*** Bug 442802 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.