Closed Bug 1040686 Opened 10 years ago Closed 10 years ago

[Homescreen] Settings support for device capability and build customization

Categories

(Firefox OS Graveyard :: Gaia::Homescreen, defect)

x86
macOS
defect
Not set
normal

Tracking

(blocking-b2g:2.0+, b2g-v2.0 fixed, b2g-v2.1 fixed)

RESOLVED FIXED
2.1 S1 (1aug)
blocking-b2g 2.0+
Tracking Status
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: kgrandon, Assigned: kgrandon)

References

Details

(Whiteboard: [systemsfe])

User Story

1) Devices with less than 512MB RAM should always get the 4 column layout and have no option in the settings to change it.
2) Devices with 512MB RAM or more should get the 3 column layout by default
3) Devices with 512MB RAM or more should be able to be configured by partners (or us if we choose to change it later for whatever reason) to be 4 columns or 3 columns
4) Devices with 512MB RAM or more should be able to have the Setting for 3 vs. 4 columns hidden from the user via a build config (that way if we choose to ship with 4 for some reason, we can hide it if need be)

Attachments

(1 file)

46 bytes, text/x-github-pull-request
arthurcc
: review+
Details | Review
For low-capability devices 3-column mode may be pixelated, so we're going to remove this for now unfortunately.

Sorry folks - if you want it you can still enable it in a local customization file.
Assignee: nobody → kgrandon
Status: NEW → ASSIGNED
Whiteboard: [systemsfe]
Target Milestone: --- → 2.0 S6 (18july)
Attached file Github pull request
Hey Arthur - could you review this one? I think I'd like to just hide this from the UI for now as it may be coming back soon. If it doesn't, then we can totally remove this code. Thanks!
Attachment #8458573 - Flags: review?(arthur.chen)
Ni? on a few UX people here for a decision. I was informed that we wanted to remove the settings from the homescreen section due to possible image degradation on 256mb devices. Several engineers have expressed concern and would like to continue dogfooding the homescreen in three columns. Can we keep the setting available for devices which have 512mb of memory and have no problems displaying three columns?
Flags: needinfo?(rmacdonald)
Flags: needinfo?(jsavory)
Flags: needinfo?(jachen)
Comment on attachment 8458573 [details] [review]
Github pull request

Clearing review until we have clarification from UX here.
Attachment #8458573 - Flags: review?(arthur.chen)
Clarification was provided over email, thanks guys.
Flags: needinfo?(rmacdonald)
Flags: needinfo?(jsavory)
Flags: needinfo?(jachen)
User Story: (updated)
Summary: [Homescreen] Remove column settings section → [Homescreen] Settings support for device capability and build customization
Comment on attachment 8458573 [details] [review]
Github pull request

Arthur - this one is now ready. Could you give me a review for this? Thanks!
Attachment #8458573 - Flags: review?(arthur.chen)
Comment on attachment 8458573 [details] [review]
Github pull request

The patch looks good to me. Could you also add unit tests? Thanks!
Attachment #8458573 - Flags: review?(arthur.chen)
(In reply to Kevin Grandon :kgrandon from comment #5)
> Comment on attachment 8458573 [details] [review]
> Github pull request
> 
> Arthur - this one is now ready. Could you give me a review for this? Thanks!

Thanks for your work here. Could you please give us a patch for v2.0 ? I want to test how much memory we are saving with this fix and bug 1040070
Flags: needinfo?(kgrandon)
(In reply to Tapas Kumar Kundu from comment #7)
> (In reply to Kevin Grandon :kgrandon from comment #5)
> > Comment on attachment 8458573 [details] [review]
> > Github pull request
> > 
> > Arthur - this one is now ready. Could you give me a review for this? Thanks!
> 
> Thanks for your work here. Could you please give us a patch for v2.0 ? I
> want to test how much memory we are saving with this fix and bug 1040070

Hi Tapas. Please note that this particular bug does not save any homescreen memory. It is actually only for hiding an option in the settings app to change the number of columns.
Flags: needinfo?(kgrandon)
Comment on attachment 8458573 [details] [review]
Github pull request

Hey Arthur - could you take another look? I addressed your nit and added a simple test to ensure we show the section based on setting. More in-depth testing is a bit tricky due to the fact that this logic is masked in a constructor. Not sure if this is a typical pattern that you guys use. I'd like to see if we can find a way to test this inside a marionette test in the future, but to do so we'd need to be able to mock the values of navigator.getFeature() if we can't yet.
Attachment #8458573 - Flags: review?(arthur.chen)
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking?]
Target Milestone: 2.0 S6 (18july) → 2.1 S1 (1aug)
Comment on attachment 8458573 [details] [review]
Github pull request

Tests ensuring the UI does get updated based on the preference is sufficient. r=me, thanks!
Attachment #8458573 - Flags: review?(arthur.chen) → review+
Product will want to block on this. Since we've gone to 4 columns for 256mb memory, we need these configuration options for the settings app.

Master: https://github.com/mozilla-b2g/gaia/commit/66f3067f1466acbb3fdd228d8f7d610752f66506
Status: ASSIGNED → RESOLVED
blocking-b2g: --- → 2.0?
Closed: 10 years ago
Resolution: --- → FIXED
(In reply to Kevin Grandon :kgrandon from comment #11)
> Product will want to block on this. Since we've gone to 4 columns for 256mb
> memory, we need these configuration options for the settings app.
> 
> Master:
> https://github.com/mozilla-b2g/gaia/commit/
> 66f3067f1466acbb3fdd228d8f7d610752f66506

Right. I'll + this then.
blocking-b2g: 2.0? → 2.0+
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking?] → [VH-FL-blocking-][VH-FC-blocking+]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: