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

RESOLVED FIXED

Status

Firefox OS
Gaia::TestAgent
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: julienw, Assigned: julienw)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

52 bytes, text/x-github-pull-request
yurenju
: review+
snowmantw
: feedback+
Details | Review | Splinter Review
46 bytes, text/x-github-pull-request
yurenju
: review+
Details | Review | Splinter Review
(Assignee)

Description

5 years ago
The goal of this bug is to restore some of the changes made in bug 940969 and that were removed by bug 943807.
(Assignee)

Comment 1

5 years ago
Actually bug 943807 has not landed on the test-agent repository, so changing the title.
Summary: [test-agent] more CSS Tweaks → [test-agent] apply bug 943807 to the js-test-agent repository
(Assignee)

Comment 2

5 years ago
Created attachment 8347333 [details] [review]
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?
Attachment #8347333 - Flags: feedback?(jlal)
Attachment #8347333 - Flags: feedback?(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!
Attachment #8347333 - Flags: feedback?(gweng) → feedback+
(Assignee)

Comment 4

5 years ago
Created attachment 8347665 [details] [review]
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?
Attachment #8347665 - Flags: review?(yurenju.mozilla)
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.
Attachment #8347665 - Flags: review?(yurenju.mozilla) → review+
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)
Attachment #8347333 - Flags: feedback?(jlal)
(Assignee)

Comment 7

5 years ago
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).
Attachment #8347333 - Flags: review?(yurenju.mozilla)
(Assignee)

Comment 8

5 years ago
Landed on master the quick fix gaia change: 0e280be0cddd79d70de34499233925957f9d4d2b
Hi julien, I'm Pretty busy today, I'll review it tomorrow.
(Assignee)

Comment 10

5 years ago
no pb, nothing urgent :)
Comment on attachment 8347333 [details] [review]
github PR

Looks good, thank you Julien!
Attachment #8347333 - Flags: review?(yurenju.mozilla) → review+
(Assignee)

Comment 12

5 years ago
master: b631701e543828b230c0624b0d59e3954398ab4e
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.