950083 - [test-agent] apply bug 943807 to the js-test-agent repository

Status: RESOLVED FIXED

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

Description

[Julien Wajsberg [:julienw]]

The goal of this bug is to restore some of the changes made in bug 940969 and that were removed by bug 943807.

Comment 1

[Julien Wajsberg [:julienw]]

Actually bug 943807 has not landed on the test-agent repository, so changing the title.

Comment 2

[Julien Wajsberg [:julienw]]

Attachment: github PR

Greg, I'd like your feedback over the CSS changes that I made to your patch. I've added a commit containing only these changes so that it's easier for you to review.

Basically, here is what I changed:
* switched to rem instead of em for nearly everything
* desktop mode
  + restored the column functionality; it was not functioning because of the "float: left" property
  + removed the "nowrap" property, I think it's better to see the whole text even if it wraps
  + adds back the blue color and the underline text decoration for .active and :hover. I find it's clearer for the user
  + adds back the padding, so that the "*" character shows correctly in ".active" mode
  + I've changed how you display the additional information in the header, using absolute positioning instead of a float.

* mobile mode
  + restored the borders, I find it's easier for the user to know where to tap 

James, I don't know how we should handle the search-tools.js file. Should it be more integrated inside test-agent, or is it ok to keep it external like this? What do you think?

Comment 3

[Greg Weng [:snowmantw][:gweng][:λ]]

Nice. I feel it's great to have your opinions listed clearly and we can keep a version with search tool & grouping, while the responsive design is respected. I must say that one of my faults is that I didn't think too much because I couldn't find out what's the purposes of the CSS tweaks you applied in the Bug 940969, and to read them directly is hard to understand (for me).

Hope we can keep making test-agent better and better!

Comment 4

[Julien Wajsberg [:julienw]]

Attachment: github PR for gaia

Hey Yuren,

I figured we can land these fixes to Gaia until we commit the full thing to the js-test-agent repository. What do you think?

Comment 5

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 8347665 [details] [review]
github PR for gaia

Now I understand what is the problems which Julien mentioned on bug 943807 and let's land it first.

Comment 6

[James Lal [:lightsofapollo]]

Comment on attachment 8347333 [details] [review]
github PR

I am PTO- generally this looked fine to me! I have very little opinion about the UI. (please take my removal of f? as a plus and move forward)

Comment 7

[Julien Wajsberg [:julienw]]

Comment on attachment 8347333 [details] [review]
github PR

Hey Yuren,

so this is the PR that incorporates Greg's changes into the test-agent.

Let's move forward and put this in like this for now (as an external plug-in) and maybe we can integrate it more inside later on (but I'm not sure this is something we want).

Comment 8

[Julien Wajsberg [:julienw]]

Landed on master the quick fix gaia change: 0e280be0cddd79d70de34499233925957f9d4d2b

Comment 9

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Hi julien, I'm Pretty busy today, I'll review it tomorrow.

Comment 10

[Julien Wajsberg [:julienw]]

no pb, nothing urgent :)

Comment 11

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 8347333 [details] [review]
github PR

Looks good, thank you Julien!

Comment 12

[Julien Wajsberg [:julienw]]

master: b631701e543828b230c0624b0d59e3954398ab4e