Closed Bug 732104 Opened 8 years ago Closed 8 years ago

Make sure BACK always takes you up a folder level in the bookmarks UI (and make that the only way to go up a folder level)

Categories

(Firefox for Android :: General, defect)

13 Branch
ARM
Android
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 14
Tracking Status
firefox13 --- verified
firefox14 --- verified
blocking-fennec1.0 --- beta+
fennec 13+ ---

People

(Reporter: nhirata, Assigned: Margaret)

Details

Attachments

(5 files, 1 obsolete file)

1. go to the awesome page after syncing
2. click on bookmarks
3. click in a folder
4. click on the history tab
5. click on the bookmarks tab
6. click the back button on the android device

Expectation : go up a folder?
Actual: exits the awesome page to content page... though this is designed behavior.

Note: 
1. looking at the folder name, it's not apparent to click on that to go up a folder level; an icon could be useful in order to signify to go up a folder level
2. my ux issue is that : It's not apparent to click on the folder name to go up the folder level after clicking on another awesomebar tab.
3. Galaxy Captivate, Nightly, 3/1/2012
Oh, I was confused by our discussion on IRC and didn't realize exactly what you were experiencing. If you're in a sub-folder, clicking the back button should always take you up a folder level, not exit the awesome screen, so this isn't working as designed. You shouldn't ever need to click on the header to go up a folder level - that's just a secret bonus feature from when I was trying to copy XUL fennec.
Assignee: nobody → margaret.leibovic
Keywords: uiwanted
Oh!  Well, I do like the secret bonus feature on top of that. 

I'm not sure though if we should be able just to go up a folder there, or exit... still would like ux input in regards to this in what they think.
tracking-fennec: --- → ?
blocking-fennec1.0: --- → ?
assigning to ian to get a UX design
Assignee: margaret.leibovic → ibarlow
tracking-fennec: ? → 13+
blocking-fennec1.0: ? → beta+
Ian - We might need some styling to make it easier to see that we can tap the element to go back up the bookmark hierarchy. If you don't think this blocks, let us know.
Back button should take you up a level.
I think Mark's right, there should be a clearer indicator that, in addition to the back button, tapping that heading should bring you up a level in the tree. 

I'll post something asap
Oof, mid-aired with Ian, but before he commented, I was going to say...

I think the bug summary needs to change here, unless we want to actually add a "go up a level" indicator to the header as proposed. Tapping the header to go up a level wasn't part of the UI spec, it was just leftover from my initial attempt to copy XUL fennec, and it seemed like there was no harm to leave it in (although I guess there is harm if it leads to confusion).

The main problem is that after switching tabs in the awesomescreen UI, the bookmark folder level seems to get lost, so using the back button fails in that case. This, I need to look into fixing, so if we're not going to change the bug summary here, maybe we should file another bug for that.

... so I guess I'll file another bug?
It sounds like there needs to be another bug.  One for the back button and one for the ui indication?
Ok, based on our IRC conversation, let's

1. Remove the ability to tap a folder heading to go back
2. Improve the look of the folder heading (coming soon from me!)
3. The Back button should always navigate up the folder tree in the Bookmarks tab, EVEN WHEN you've switched to a different tab and come back.
Updating the summary to reflect what's going on here.
Assignee: ibarlow → margaret.leibovic
Summary: It's not apparent to click on the folder name to go up the folder level after clicking on another awesomebar tab. → Make sure BACK always takes you up a folder level in the bookmarks UI (and make that the only way to go up a folder level)
Attached file mockup of header tweaks (obsolete) —
Changes:
- Match font size to tab font size
- Make heading bold
- Make heading row 20% shorter
- Make rule below heading 3x thicker
Attached image mockup of header tweaks
oops, uploaded the PSD first, which was not what I wanted to do
Attachment #605912 - Attachment is obsolete: true
What's happening is that right now we're only handling the case when BACK is pressed when the edit text is focused:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/AwesomeBar.java#163

I'm not sure why the edit text doesn't lose focus when you first tap on one of the tabs, but it loses focus after you switch to History then back to Bookmarks. Overriding onBackPressed like this does what we want (this doesn't get called in the edit text focused case).
Attachment #606009 - Flags: review?(mark.finkle)
I copied these styles from awesomebar_header_row.xml so that they'll match the history section headers. I was hoping to just use that file instead of aweseomebar_folder_header_row.xml, but we need the LinearLayout wrapped around the TextView in order to totally collapse the header in the root folder view (I remember having issues trying to figure this out at one point).
Attachment #606011 - Flags: review?(mark.finkle)
Attached image screenshot
Keywords: uiwanted
Comment on attachment 606009 [details] [diff] [review]
(1/3) Make sure BACK always takes you up a folder level

Just a note to myself. We are leaving the textbox BACK handling in place so the fullscreen keyboard situation is still handled. Otherwise, we could remove the code here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/AwesomeBar.java#177
Attachment #606009 - Flags: review?(mark.finkle) → review+
Attachment #606010 - Flags: review?(mark.finkle) → review+
Attachment #606011 - Flags: review?(mark.finkle) → review+
Looks great, Margaret!
(In reply to Mark Finkle (:mfinkle) from comment #17)
> Comment on attachment 606009 [details] [diff] [review]
> (1/3) Make sure BACK always takes you up a folder level
> 
> Just a note to myself. We are leaving the textbox BACK handling in place so
> the fullscreen keyboard situation is still handled. Otherwise, we could
> remove the code here:
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/AwesomeBar.
> java#177

Yeah, we need to keep that. That code path was introduced in the first place because we wanted to leave the awesomescreen with one tap (see bug 700951), and now in this folder case, we want to go up a folder level.
Comment on attachment 606009 [details] [diff] [review]
(1/3) Make sure BACK always takes you up a folder level

[Approval Request Comment]
Mobile-only. Blocking fennec-1.0.
Attachment #606009 - Flags: approval-mozilla-aurora?
Comment on attachment 606010 [details] [diff] [review]
(2/3) Remove the ability to tap a folder heading to go back

[Approval Request Comment]
Mobile-only. Blocking fennec-1.0.
Attachment #606010 - Flags: approval-mozilla-aurora?
Comment on attachment 606011 [details] [diff] [review]
(3/3) Style header row

[Approval Request Comment]
Mobile-only. Blocking fennec-1.0.
Attachment #606011 - Flags: approval-mozilla-aurora?
SoftVision, please update any tests with this change.
Flags: in-litmus?(fennec)
Comment on attachment 606009 [details] [diff] [review]
(1/3) Make sure BACK always takes you up a folder level

[Triage Comment]
Mobile only - approved for Aurora 13.
Attachment #606009 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #606010 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #606011 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified/fixed on:

Aurora Fennec 13.0a2 (2012-03-26)
Nightly Fennec 14.0a1 (2012-03-26)
Device: Samsung Nexus S
OS: Android 2.3.6

Testcase added in Litmus
https://litmus.mozilla.org/show_test.cgi?id=53247
Status: RESOLVED → VERIFIED
Flags: in-litmus?(fennec) → in-litmus+
You need to log in before you can comment on or make changes to this bug.