1254555 - Unnecessary scroll to reveal full first run panel and options

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: First Run, defect)

Description

[Ioana Chiorean]

Android 2.3.5 - HTC Desire HD
Fennec 46 Beta 1

Steps:
1. Open Fennec with a clean profile on a low resolution device

Expected Result: 
- you should be able to see the 'Next' link at the bottom part of the panel/screen

Actual Result:
- you can not see the Next button and some users might be blocked by that ( they can use the top area to switch to other panel)

Notes:
- new first run: https://www.youtube.com/watch?v=cfykZ9wEbQI&feature=youtu.be
- old first run: https://www.youtube.com/watch?v=5dZDNDTqsR0&feature=youtu.be

Comment 1

[Chenxia Liu [:liuche]]

Thanks for the videos Ioana.

I think we can do a couple of things with this, either:
- Add a spacer with a weight in between the text body
- Have a specific layout for small devices
- WONTFIX this

I'll add a mentor flag to this bug - basically, we want this to show the "Next" text on smaller-screen devices. Let's try this by adding a spacer with a weight between the "Next" item and the body text, and trying to make it as close to the current version as possible.

Comment 2

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

I'm not sure what "adding a spacer" means but some screenshots on how this "spacer" affects most devices would be helpful :)

Comment 3

[Alex]

Hi,
I'am really interesting by this bug ( it will be my first bug)
But i don't understand how to put my emulator (mozemulator) to a low quality =/

Do you know how to do that?

Comment 4

[Chenxia Liu [:liuche]]

Hi Alex, do you use Android Studio or IntelliJ? You should be able to create new emulators in Tools > Android > AVD Manager (or just shift-shift to bring up the search and type "AVD Manager"). From there you should be able to create an emulator that will show up in the device chooser when you want to run a build.

As a note, this probably isn't a ldpi device, an mdpi one would do - we want the case where there is space to show all the content, but we're not.

Additionally, make sure you test any changes you make on normal size devices and make sure that we're not regressing anything for the other devices.

Let me know if you have any questions!

Comment 5

[Alex]

Hi Chenxia, Thank you for your answer.
I'am using IntelliJ.
I succes to recreat the bug on a QGVA 320 * 480 mdpi, with marshmallow Android 6.0x86_64.
Can you just help me a bit where is situated the code of this bug? Or an indication ? ^.^

Comment 6

[Alex]

Hello,
Is it possible to be assigned to this bug?

Comment 7

[Alex]

(I found where is the problem, I 'am working on it  :)  )

Comment 8

[Chenxia Liu [:liuche]]

Thanks Alex! We don't assign bugs until there's a patch, because we often get lots of interest but not much followup.

If you haven't seen it already, this is our guideline to writing/preparing patches. Let me know if you have questions, and thanks again for investigating this bug!
https://wiki.mozilla.org/Mobile/Fennec/Android/Suggested_workflow#Creating_commits_and_submitting_patches

Comment 9

[Alex]

Okey, I will do my best to fix this bug then :)

Comment 10

[Alex]

Hi,
I'm actually trying to fix the UI bug, but I can't find an easy way to launch the firstscreen panel.
Do you know an easy way to do it (maybe through Intent ?).

Comment 11

[Alex]

Attachment: Patch for Bug 1254555

Add files for multiples densities
Corrects the bug for mdpi and hdpi screens

Comment 12

[Alex]

Hello,
Is it possible to have a feedback about my patch ? :)

Comment 13

[Kevin Brosnan [Ex-Mozilla]]

All of the primary developers are at Google I/O this week.

Comment 14

[Alex]

oh, okey thank you for the information

Comment 15

[Chenxia Liu [:liuche]]

Comment on attachment 8749272 [details] [diff] [review]
Patch for Bug 1254555

Review of attachment 8749272 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the delay!

Hmm, so looking at this, I don't think that we want to do this by hard-coding specific dimensions for different dpi screens. I don't think this would work for all devices, so we'd just be pushing the problem off to a smaller set of specific devices. Specifically, there should be a way to use layout_weight or perhaps RelativeLayout to constrain these layouts so they don't take up too much space, so I'd like to see an approach that doesn't rely on hard-coded dimensions.

::: mobile/android/base/resources/values-hdpi/dimens.xml
@@ +4,5 @@
> +   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> +
> +<resources>
> +
> +    <dimen name="firstrun_content_width">300dp</dimen>

If this value isn't changing between versions, you don't need to include it in a dpi-specific file.

Comment 16

[Shubham]

Hi, Can I take up this bug(this will be my first bug) from where Alex left.

Comment 17

[Mike Hoye [:mhoye]]

Shubham: looks like it's been sitting idle for a bit. Unless Alex says he'd like to take another run at it, go for it.

Comment 18

[Shubham]

(In reply to Alex from comment #14)
> oh, okey thank you for the information

Hey Alex, are you still working on it. If not would it be okay if I take it over?

Comment 19

[Shubham]

Alex ping for this.

Comment 20

[Alex]

Hey, yeah you can do it, sorry for delay

Comment 21

[Shubham]

Attachment: Bug 1254555 - Unnecessary scroll to reveal full first run panel.

Review commit: https://reviewboard.mozilla.org/r/80202/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/80202/

Comment 22

[Shubham]

Hi Mike, I have pushed the patch for the bug. Looking forward to your feedback. Thanks.

Comment 23

[Mike Hoye [:mhoye]]

Thanks, Subham, but I'm not the right person. Chenxia, what do you think?

Comment 24

[Shubham]

Hi Chenxia, did you get a chance to look at the patch.

Comment 25

[Chenxia Liu [:liuche]]

Comment on attachment 8793470 [details]
Bug 1254555 - Unnecessary scroll to reveal full first run panel.

Thanks Shubham, I took a look at this patch! The code looks good, but there's the problem when no switch is present and the phone is in landscape, and the "Next" text overlaps the text of the slide. Also, there's the case where the "Next" button is actually above the switch. I'll add screenshots of these two cases.

You could consider setting the switch view to invisible, and adding it to the RelativeLayout positioning hierarchy.

Alternatively, if you find it isn't actually easy to take the approach to fix this, we should just WONTFIX it.

Thanks for your help!

Comment 26

[Chenxia Liu [:liuche]]

Attachment: Screenshot: Overlap

Comment 27

[Chenxia Liu [:liuche]]

Attachment: Screenshot: Incorrect view ordering

Comment 28

[mddrill]

Hi, how can I work on this bug? I'm new to open source.

Comment 29

[Shubham]

(In reply to Chenxia Liu [:liuche] from comment #25)
> Comment on attachment 8793470 [details]
> Bug 1254555 - Unnecessary scroll to reveal full first run panel.
> 
> Thanks Shubham, I took a look at this patch! The code looks good, but
> there's the problem when no switch is present and the phone is in landscape,
> and the "Next" text overlaps the text of the slide. Also, there's the case
> where the "Next" button is actually above the switch. I'll add screenshots
> of these two cases.
> 
> You could consider setting the switch view to invisible, and adding it to
> the RelativeLayout positioning hierarchy.
> 
> Alternatively, if you find it isn't actually easy to take the approach to
> fix this, we should just WONTFIX it.
> 
> Thanks for your help!

Hi Chenxia,
As of with the current structure, it is not easy to fix. One approach could be decreasing the padding between layout elements for the low resolution devices but as you already mentioned that would be pushing the problem off to a smaller set of specific devices.

Comment 30

[Chenxia Liu [:liuche]]

Thanks for looking into this, Shubham, because this bug has been open for a while.

I think the best course of action here is to WONTFIX this bug. I tried this out on a bunch of small devices, and for the most part with the newest firstrun experiment, this problem shows up rarely and it's not worth having more people invest time into this.

Thanks again!