1203456 - Factorize the headers into 1 class

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::UI Tests, defect)

Description

[Johan Lorenzo [:jlorenzo]]

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

[GitHub Autolander Bot]

Attachment: [gaia] mwargers:1203456 > mozilla-b2g:master

Comment 2

[Martijn Wargers (dead)]

Comment on attachment 8666397 [details] [review]
[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.

Comment 3

[No-Jun Park [:njpark]]

Comment on attachment 8666397 [details] [review]
[gaia] mwargers:1203456 > mozilla-b2g:master

This looks like a pretty good idea to me. r+

Comment 4

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8666397 [details] [review]
[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.

Comment 5

[Martijn Wargers (dead)]

Comment on attachment 8666397 [details] [review]
[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.

Comment 6

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8666397 [details] [review]
[gaia] mwargers:1203456 > mozilla-b2g:master

Clearing the review, now that the workaround due to the shadow dom, is not necessary anymore.

Comment 7

[Martijn Wargers (dead)]

I'll wait on bug 1211679 to be checked in, then.

Comment 8

[Martijn Wargers (dead)]

Comment on attachment 8666397 [details] [review]
[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.

Comment 9

[Paul Yang [:pyang] (away)]

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.

Comment 10

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8666397 [details] [review]
[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.

Comment 11

[Martijn Wargers (dead)]

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