1015479 - Reorder gaia-header children to logical order

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Eitan Isaacson [:eeejay]]

Currently the markup order, and the visual order differ.
Visually it is [action button][title][menu buttons], but in the markup it is [action button][menu buttons][title]. This is easily remedied with the new flex css!

Comment 1

[Eitan Isaacson [:eeejay]]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19613

Comment 2

[Kevin Grandon :kgrandon]

I wonder if we should add a unit test which will validate the ordering of the elements?

Comment 3

[Yan Or]

Comment on attachment 8428137 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19613

Adding Arnau as he's better fitted to review the CSS than me.

Comment 4

[Arnau March [:arnau] ( not working in Firefox OS anymore :( )]

Comment on attachment 8428137 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19613

Eitan, the layout looks broken :(
Please check: /shared/elements/gaia_header/examples/index.html
I'll check tomorrow with Wilson if we could change the template, to remove the menu. That would fix your patch.

Comment 5

[Wilson Page [:wilsonpage]]

IMO the structure of Shadow-DOM elements is meaningless. The Shadow-DOM is is meant to be purely structural so that the Light-DOM can become elegant and semantic.

For example: screen-readers wouldn't parse the structural internals of the <video> element controls. My understanding is, Shadow-DOM is the internal/hidden implementation of a component and shouldn't ever be used to derive semantics.

Comment 6

[Marco Zehe (:MarcoZ)]

Wilson, this is not the case. We need to expose the semantics of the shadow DOM elements to provide screen readers with the knowledge about the UI elements of the composit widget. For example, without parsing the shadow DOM of the video element, that video element would be nothing more than a blackbox. But the buttons you interact with like Play/Pause, Mute, the sliders and their information, need to be exposed to screen readers as well so the user knows if the video is playing, how far it has progressed, what volume it's at. The video element alone doesn't tell screen readers that. The same goes for web components. Information about those has to be exposed so the actual items to interact with or that provide other kinds of info can be made accessible.

In fact, with the canvas element, the shadow DOM is the *only* way to make contents within it accessible.

Comment 7

[Arnau March [:arnau] ( not working in Firefox OS anymore :( )]

Eitan,
please change in script.js the template to:
template.innerHTML = '<header id="header">' +
      '<button id="action-button"></button>' +
      '<content select="h1,button,a"></content>' +
    '</header>';

And will work the way you expect, as in your PR buttons in the menu appear one on top of the other.
Using flex we don't need the menu anymore.

Comment 8

[Eitan Isaacson [:eeejay]]

Comment on attachment 8428137 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19613

Updated the markup to your suggestion and removed the menu selector in the css.

Comment 9

[Arnau March [:arnau] ( not working in Firefox OS anymore :( )]

Comment on attachment 8428137 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/19613

Much more flexible component now!
Thanks Eitan.

Comment 10

[Eitan Isaacson [:eeejay]]

https://github.com/mozilla-b2g/gaia/commit/06e7fcde1627d741e06a32bce725f45ff2678953

Sorry for the delay, took travis a while to become green.