Closed Bug 1209038 Opened 9 years ago Closed 9 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: gabriel, Assigned: kaze)

References

Details

Attachments

(1 file)

CSS refactoring of the 'headers' building block for LTR/RTL use.
Assignee: nobody → gabriel
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8667795 - Flags: review?(pivanov)
Comment on attachment 8667795 [details] [review]
[gaia] Phoxygen:bug1209038-bb-headers > mozilla-b2g:master

I will let Wilson to r+ this one
Attachment #8667795 - Flags: review?(pivanov) → review?(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.
Attachment #8667795 - Flags: review?(wilsonpage)
(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 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.
Attachment #8667795 - Flags: review?(pivanov)
Comment on attachment 8667795 [details] [review]
[gaia] Phoxygen:bug1209038-bb-headers > mozilla-b2g:master

Sure :) r+ LGTM :)
Attachment #8667795 - Flags: review?(pivanov) → review+
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.
Assignee: gabriel → kaze
This bug blocks RTL related bugs. Maybe we can open another bug for replacing bb-header by gaia-header, and close this one?
(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.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: