1209038 - (Gaia RTL 2.5) CSS refactoring: [bb] headers

Status: RESOLVED FIXED

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

Description

[gabriel]

CSS refactoring of the 'headers' building block for LTR/RTL use.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] Phoxygen:bug1209038-bb-headers > mozilla-b2g:master

Comment 2

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Comment on attachment 8667795 [details] [review]
[gaia] Phoxygen:bug1209038-bb-headers > mozilla-b2g:master

I will let Wilson to r+ this one

Comment 3

[Wilson Page [:wilsonpage]]

Comment on attachment 8667795 [details] [review]
[gaia] Phoxygen:bug1209038-bb-headers > mozilla-b2g:master

I've never worked on the building-blocks so don't feel like I can r+ this. I'm actually quite surprised how many apps are still using headers.css!

- apps/bluetooth/settings.html
- apps/bookmark/save.html
- apps/collection/view.html
- apps/communications/contacts/views/matching/matching_contacts.html
- apps/costcontrol/index.html
- apps/default_theme/index.html
- apps/email/index.html
- apps/keyboard/settings.html
- apps/privacy-panel/index.html
- apps/settings/index.html
- shared/pages/import/curtain.html
- shared/pages/import/import.html
- tv_apps/dlna-player/index.html
- tv_apps/fling-sender/index.html
- tv_apps/notification-sender/index.html

I wonder if this patch can migrate some of theses apps to <gaia-header> whcih has already fixed RTL.

NOTE: Some of these apps may be using header.css for the sub-header styling not the primary app bar.

Comment 4

[Fabien Cazenave [:kaze]]

(In reply to Wilson Page [:wilsonpage] from comment #3)
> I wonder if this patch can migrate some of theses apps to <gaia-header>
> whcih has already fixed RTL.

This is tempting, but it’s a separate issue: we need this headers.css building block ASAP because it’s part of the RTL navigation support, along with System and <gaia-header>. The Settings app refactoring is blocked by this building block right now.

Besides, it’s already hard enough to find a reviewer for this, and adding patches on other apps won’t help. :-)

Suggestion: let’s land this first building block patch and keep this bug open so that we can migrate (some of) the apps you listed to the <gaia-header> component: one patch per app, one peer per patch.

Comment 5

[Fabien Cazenave [:kaze]]

Comment on attachment 8667795 [details] [review]
[gaia] Phoxygen:bug1209038-bb-headers > mozilla-b2g:master

Pavel, may I insist? ;-)

I kinda remember we started this building block together, so I really can’t think of any better reviewer than you for this patch.

Comment 6

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Comment on attachment 8667795 [details] [review]
[gaia] Phoxygen:bug1209038-bb-headers > mozilla-b2g:master

Sure :) r+ LGTM :)

Comment 7

[Fabien Cazenave [:kaze]]

Thanks Pavel! Merged in master: https://github.com/mozilla-b2g/gaia/commit/b7623c3

Leaving this bug open so we can attach new patches to migrate Gaia apps to the <gaia-header> web component, as per comment 3.

Comment 8

[Augustin Trancart [:autra]]

This bug blocks RTL related bugs. Maybe we can open another bug for replacing bb-header by gaia-header, and close this one?

Comment 9

[Sam Foster [:sfoster] (he/him)]

(In reply to Augustin Trancart [:autra] from comment #8)
> This bug blocks RTL related bugs. Maybe we can open another bug for
> replacing bb-header by gaia-header, and close this one?

Agreed, and doing that migration should be done on an app by app basis. We can make a meta for that, I'll let Wilson follow-up.