Last Comment Bug 734915 - Replace the feed subscribe folderpicker with new methods
: Replace the feed subscribe folderpicker with new methods
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Feed Reader (show other bugs)
: unspecified
: All All
: -- normal (vote)
: Thunderbird 14.0
Assigned To: alta88
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-12 10:00 PDT by alta88
Modified: 2012-11-10 11:14 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch. (15.86 KB, patch)
2012-03-12 10:13 PDT, alta88
no flags Details | Diff | Splinter Review
patch. (15.91 KB, patch)
2012-03-12 10:36 PDT, alta88
no flags Details | Diff | Splinter Review
patch. (20.38 KB, patch)
2012-03-12 17:34 PDT, alta88
no flags Details | Diff | Splinter Review
patch. (20.33 KB, patch)
2012-03-14 14:13 PDT, alta88
no flags Details | Diff | Splinter Review
patch. (22.42 KB, patch)
2012-03-15 16:13 PDT, alta88
mozilla: review+
Details | Diff | Splinter Review

Description alta88 2012-03-12 10:00:14 PDT

    
Comment 1 alta88 2012-03-12 10:13:18 PDT
Created attachment 604993 [details] [diff] [review]
patch.

1. replace old rdf rules to newer binding based picker.
2. show path not just leaf folder name.
3. when folder selected, allow click/enter toggle to display folder pretty path and absolute file path.
4. ensure keyboard works for all actions.
Comment 2 alta88 2012-03-12 10:36:47 PDT
Created attachment 604999 [details] [diff] [review]
patch.

tweak..
Comment 3 alta88 2012-03-12 17:34:32 PDT
Created attachment 605242 [details] [diff] [review]
patch.

5. 'loading' message for feedback on long tree builds.

final tweak...
Comment 4 Ian Neal 2012-03-14 07:13:40 PDT
Comment on attachment 605242 [details] [diff] [review]
patch.

Just a quick drive-by.

>+  setFolderPicker: function(aFolder)
>   {
>     let selectFolder = document.getElementById("selectFolder");
>+    let selectFolderPopup = document.getElementById("selectFolderPopup");
>+    let selectFolderValue = document.getElementById("selectFolderValue");
These 3 definitions could probably be moved after the early return as they're not needed until after then.
>     let editFeed = document.getElementById("editFeed");
> 
>+    let folderPrettyPath = FeedUtils.getFolderPrettyPath(aFolder);
>+    if (!folderPrettyPath)
>       return editFeed.disabled = true;
> 
>+    selectFolderPopup.selectFolder(aFolder);
As this is only used the once you could probably inline it and remove selectFolderPopup definition.


>+  getFolderPrettyPath: function(aFolder) {
>+    let folderPrettyPath = null;
>+    let msgFolder = GetMsgFolderFromUri(aFolder.URI, true);
>+    if (!msgFolder)
>+      // Not a real folder uri.
>+      return folderPrettyPath;
Just return null here, makes it more obvious and avoids having to create the variable folderPrettyPath.

>+    // Leaf folder last.
>+    pathParts.push(aFolder.name);
>+    folderPrettyPath = decodeURI(pathParts.join(" / "));
Just return the decodeURI here rather setting it to the variable and returning that.
>+
>+    return folderPrettyPath;
Comment 5 alta88 2012-03-14 14:13:09 PDT
Created attachment 605945 [details] [diff] [review]
patch.


sure.
Comment 6 alta88 2012-03-15 16:13:40 PDT
Created attachment 606396 [details] [diff] [review]
patch.


final final tweak, for real.  (make keyboard folderpicking/update work for non visible folders.)
Comment 7 Ryan VanderMeulen [:RyanVM] 2012-03-18 10:21:56 PDT
http://hg.mozilla.org/comm-central/rev/70af4549f05b

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