Closed Bug 1013848 Opened 9 years ago Closed 5 years ago

[Tablet] Support device type css selection during build time

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: atsai, Unassigned)

References

Details

(Whiteboard: [Vixen][TCP=feature-p1])

Attachments

(1 file, 2 obsolete files)

60.53 KB, image/png
Details
** The case is by the testing on Vixen Tablet devices. It might not be able to reproduce on other mobile devices **

Layout for sound setting is mess up. Please check the attachment.

Gaia      7c55cc2baabc2d66d512768e79b9cbc67bb83040
Gecko     d6d67da827d0180b95db2592b725321f6102d141
BuildID   20140520190311
Version   32.0a1
ro.build.version.incremental=eng.waynechen.20140520.184405
ro.build.date=二  5月 20 18:44:35 CST 2014
Blocks: vixen
Summary: [Vixen][Tablet] → [Vixen][Tablet] Settings layout is mess up
Tested w/ local build based on

- gaia  78977eb8d43beed1bad709a70cee3b0c4293056d
- gecko 96c549d1054014ea76d8b079f36164eb437261c0

and local patches for BT and image thumbnails.

It's still reproducible
Whiteboard: [Vixen] → [Vixen][TCP=feature-p1]
Assignee: nobody → shchen
Summary: [Vixen][Tablet] Settings layout is mess up → [Tablet] Support device type css selection during build time
Attached file WIP (obsolete) —
Once we have bug 1024893 landed, we have the ability to choose css by device type during build time. Maybe one more thing we can do is to eliminate media querying device width and determine to load settings_large.css or not.

This patch simply insert settings.css before settings_large.css, also remove media query part like |@media (min-width: 768px)|. I can polish css later to make this more efficient.

How do you think?
Attachment #8471327 - Flags: feedback?(arthur.chen)
Attached file WIP 2 (obsolete) —
Attachment #8471354 - Flags: feedback?(arthur.chen)
What we landed in bug 1024893 is actually a workaround for bug 997101, so I'm not sure if it should be kept. And separating the styles for different devices means double efforts maintaining the styles. What is the long term goal regarding this part?
Flags: needinfo?(shchen)
We do have a plan to introduce css preprocessor in respond to different device css, but consider we also need to take care people that still using Nightly as their main development tool, css preprocessor seems not a proper solution for it.

So I think bug 1024894 is a good start to solve this kind of problem, not sure if we have any concern on this one, maybe I can also address those concern in this patch.
Flags: needinfo?(shchen)
I think the major concern would be maintaining two or even more duplicate styles requires effort. The decision should be made based on the types of the devices that we are going to support and the difference among the styles they use. If they share most of the styles then I would prefer what use currently, overriding the styles for specific devices. On the other hand, if there are going to be a totally different UI for the coming devices, then using separating styles makes more sense.
I agree with your point that we should take shared styles into consideration. So I have provide two different patch, WIP 1 completely separate the style and WIP 2 override shared ones.

And with bug 1024894 we can achieve both, it can used for removing tablet css from phone build, it could also used for removing phone css from tablet build, and that's for separating case. If we keep phone css for tablet build, then it works for overriding case. 

So I was thinking keep bug 1024894 and build some device related optimization on top of it make sense, what do you think?
Comment on attachment 8471354 [details] [review]
WIP 2

Thanks for the explanation!

Ideally we should extract the shared styles from the styles for phones and tablets, then we can add phone or tablets specific styles based on the device type in build time. But extracting the shared styles is an extremely difficult work and using the phone styles as the shared styles seems more feasible, so I am going to f+ this patch.
Attachment #8471354 - Flags: feedback?(arthur.chen) → feedback+
Attachment #8471327 - Flags: feedback?(arthur.chen)
Attachment #8471327 - Attachment is obsolete: true
Attachment #8471354 - Attachment is obsolete: true
Unassign myself since I don't have enough bandwidth on it.
Assignee: shchen → nobody
Firefox OS is not being worked on
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.