feat(headers): rename and update doc --INNO-662 #234
Conversation
|
|
||
| ## Why and how to use this component | ||
| Users need a way to easily identify the branding, to search and change the language if needed. | ||
| the page title functions as the top level H1. |
There was a problem hiding this comment.
Please either format as un-ordered list (bullets) as in the docs or start with capital letters on new sentences.
Currently it's like a single paragraph with sentences that does not start with capital letters:
...
Users need a way to easily identify the branding, to search and change the language if needed. the page title functions as the top level H1. it is also used to construct the
...
| <div class="ecl-page-header__body"> | ||
| <!-- Site identification (optional) --> | ||
| <div class="ecl-page-header__identity"> | ||
| Digital single market |
There was a problem hiding this comment.
Would be better if this comes comes from the context.
There was a problem hiding this comment.
I didn't plan to change the component originally (the ticket was more about updating the doc and changing the name of variants) but your comment convinced me to refactor it.
| <!-- Meta items (optional) --> | ||
| <div class="ecl-page-header__meta"> | ||
| <div class="ecl-meta ecl-meta--header"> | ||
| <span class="ecl-meta__item ecl-u-f-up">News article</span> |
There was a problem hiding this comment.
Please include dynamically, if possible.
| module.exports = { | ||
| title: 'Basic', | ||
| label: 'Basic', | ||
| handle: 'ecl-page-headers--basic', |
There was a problem hiding this comment.
I can't open preview windows on basic and default variants, probably because of the handlers.
https://drive.google.com/file/d/0B8knVVW86wNgV3JfMm0xRlFmQU0/view
There was a problem hiding this comment.
It covers the scope of the ticket - @degliwe thanks for consolidating the logic in 1 template!
I think it's ok to leave the ecl-page-header--image rule in the css as it's probable that there's a change in mind and this feature comes back later.
|
|
||
| {% if meta is defined %} | ||
| <div class="ecl-page-header__meta"> | ||
| <div class="ecl-meta ecl-meta--header"> |
There was a problem hiding this comment.
I think we can create a follow-up ticket to improve the @ec-europa/ecl-meta to be to use it here as:
{% include '@ec-europa/ecl-meta' with {
'modifier': 'ecl-meta--header',
'meta_item_type_data': meta.type,
'meta_item_data': '....',
} %}
However, the component is not ready to have multiple ecl-meta__item and cannot accommodate date-display-single at the moment, which is definitely out of scope.
There was a problem hiding this comment.
Follow-up ticket INNO-668
rename headers, update doc, remove unused headers