Closed
Bug 1203456
Opened 9 years ago
Closed 9 years ago
Factorize the headers into 1 class
Categories
(Firefox OS Graveyard :: Gaia::UI Tests, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: jlorenzo, Assigned: martijn.martijn)
References
Details
Attachments
(1 file)
Like started in bug 1202388 and discussed offline, the headers are good candidates to be factorized. Like for switches, let's define a general abstract classes and subclasses for regular headers and GaiaHeaders.
Comment 1•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → martijn.martijn
Assignee | ||
Comment 2•9 years ago
|
||
Comment on attachment 8666397 [details]
[gaia] mwargers:1203456 > mozilla-b2g:master
Like this? I did testing on smoketests, settings tests, test_settings_PS_RTL.py and test_settings_personalization_RTL.py.
Attachment #8666397 -
Flags: review?(npark)
Attachment #8666397 -
Flags: review?(jlorenzo)
Comment 3•9 years ago
|
||
Comment on attachment 8666397 [details]
[gaia] mwargers:1203456 > mozilla-b2g:master
This looks like a pretty good idea to me. r+
Attachment #8666397 -
Flags: review?(npark) → review+
Reporter | ||
Comment 4•9 years ago
|
||
Comment on attachment 8666397 [details]
[gaia] mwargers:1203456 > mozilla-b2g:master
That's a good start, but I think there are a couple of mistakes that might turn some tests to be intermittent. More details in the PR.
Attachment #8666397 -
Flags: review?(jlorenzo)
Assignee | ||
Comment 5•9 years ago
|
||
Comment on attachment 8666397 [details]
[gaia] mwargers:1203456 > mozilla-b2g:master
I followed some of your review comments, but not all.
Regards your _tap_and_wait_for_element_to_disappear suggestion, there are certain similarities, but there are also 2 differences and that would make it odd to combine these 2 instances and not more readable than it is now, I think.
Let me know what you think.
Attachment #8666397 -
Flags: review?(jlorenzo)
Reporter | ||
Comment 6•9 years ago
|
||
Comment on attachment 8666397 [details]
[gaia] mwargers:1203456 > mozilla-b2g:master
Clearing the review, now that the workaround due to the shadow dom, is not necessary anymore.
Attachment #8666397 -
Flags: review?(jlorenzo)
Assignee | ||
Comment 7•9 years ago
|
||
I'll wait on bug 1211679 to be checked in, then.
Depends on: 1211679
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8666397 [details]
[gaia] mwargers:1203456 > mozilla-b2g:master
Now added the switch_to_shadow_root stuff.
Sorry, I had to get to latest master to get support for switch_to_shadow_root. For that I rebased everything.
Attachment #8666397 -
Flags: review?(jlorenzo)
Comment 9•9 years ago
|
||
Just thought class GaiaHeader doesn't work like bug description.
from the description I suppose to use it like
class PageWithHeader(Base, GaiaHeader):
and you can call
PageWithHeader.go_back()
without extra wrapping.
Reporter | ||
Comment 10•9 years ago
|
||
Comment on attachment 8666397 [details]
[gaia] mwargers:1203456 > mozilla-b2g:master
Paul has a great point! Inheriting from PageWithHeader is a good thing. We couldn't do it with the GaiaSwitch and such because we might have many switches in one single page.
I think the PR is good as is, and we can perform the inheritance in a follow up. Unless you want to handle it in this PR, Martijn.
Attachment #8666397 -
Flags: review?(jlorenzo) → review+
Assignee | ||
Comment 11•9 years ago
|
||
Regarding comment 9 and comment 10, I'm not exactly sure how/what to implement, so I merged this and I'll file a follow-up bug regarding those comments.
Merged: https://github.com/mozilla-b2g/gaia/commit/fd8ab406757e3126807d1c76ebee8dd159273a3b
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Reporter | ||
Updated•9 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•