Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OPENEUROPA-1695: Implement footer block. Apply ECL component #186

Merged
merged 16 commits into from
Mar 19, 2019

Conversation

sergepavle
Copy link
Member

OPENEUROPA-1695

Description

Change log

  • Added:
  • Changed:
  • Deprecated:
  • Removed:
  • Fixed:
  • Security:

Commands

[Insert commands here]

@sergepavle sergepavle marked this pull request as ready for review March 7, 2019 16:38
@imanoleguskiza imanoleguskiza changed the base branch from master to OPENEUROPA-1693 March 8, 2019 12:07
imanoleguskiza
imanoleguskiza previously approved these changes Mar 11, 2019
composer.json Outdated Show resolved Hide resolved
language switcher: ".ecl-language-list"
language switcher link: ".ecl-lang-select-sites__link .ecl-lang-select-sites__label"
language switcher overlay: ".ecl-dialog__body"
logo: "a.ecl-logo"
page header: ".ecl-page-header__body"
priorities dropdown menu: "#nav-menu-expandable-group-3 .ecl-navigation-menu__links"
search box: ".demo-block-header-search"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be out of scope, why are we removing it here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this PR missed attention: #176
So I did the same within this PR.

config/optional/block.block.oe_theme_corporate_footer.yml Outdated Show resolved Hide resolved
modules/oe_theme_demo/README.md Outdated Show resolved Hide resolved
oe_theme.theme Outdated Show resolved Hide resolved
oe_theme.theme Outdated Show resolved Hide resolved
@sergepavle sergepavle changed the title OPENEUROPA-1695: Implement footer block. Apply ECL pattern OPENEUROPA-1695: Implement footer block. Apply ECL component Mar 11, 2019
@sergepavle sergepavle changed the base branch from OPENEUROPA-1693 to master March 12, 2019 08:42
@sergepavle sergepavle force-pushed the OPENEUROPA-1695 branch 4 times, most recently from 6c255e9 to 76b6453 Compare March 15, 2019 11:14
* Theme override for the corporate block footer.
*
* Available variables:
* - content: rendered footer pattern. *
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small typo at end of line * - content: rendered footer pattern. *

use Symfony\Component\DomCrawler\Crawler;

/**
* Class CorporateBlocksFooterTest.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add meaningful class description.

'label_display' => '0',
];

$render = $this->buildBlock('oe_footer', $config);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should override here the block footer configuration object by providing test configuration, so we can make sure that the template styled the footer correctly. I propose to have just 2 links per link category and test labels.

$actual = $crawler->filter('footer.ecl-footer div.ecl-footer__corporate-top div.ecl-row div.ecl-footer__column');
$this->assertCount(3, $actual);
$actual = $crawler->filter('footer.ecl-footer div.ecl-footer__corporate-bottom div.ecl-row ul.ecl-footer__list li.ecl-footer__list-item');
$this->assertCount(7, $actual);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following the comment above we should add here assertions that assert links and labels are styled correctly.

@upchuk upchuk merged commit f61cb37 into master Mar 19, 2019
@upchuk upchuk deleted the OPENEUROPA-1695 branch March 19, 2019 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants