-
Notifications
You must be signed in to change notification settings - Fork 12
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
OEL-1915: Support date in card pattern. #291
Conversation
$day_month = $expected['day'] . ' ' . $expected['month']; | ||
|
||
if (isset($expected['end_day']) && isset($expected['end_month'])) { | ||
$day_month .= ' - ' . $expected['end_day'] . ' ' . $expected['end_month']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't want to do extra processing here nor patch the template just for this strange looking extra space
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it's too risky like this unfortunately, we need to clean it up.
*/ | ||
protected function assertCardImage($expected_image, string $variant, Crawler $crawler): void { | ||
if ($variant === 'search') { | ||
$image_div = $crawler->filter('.row .col-md-3 img.card-img-top'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the assertions are the same, can we just determine the selector in the if() ?
* The DomCrawler where to check the element. | ||
*/ | ||
protected function assertDate(array $expected, Crawler $crawler): void { | ||
// @todo Use dedicated pattern assert once we re-work date_plock pattern. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
date_block
$day_month = $expected['day'] . ' ' . $expected['month']; | ||
|
||
if (isset($expected['end_day']) && isset($expected['end_month'])) { | ||
$day_month .= ' - ' . $expected['end_day'] . ' ' . $expected['end_month']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it's too risky like this unfortunately, we need to clean it up.
* @return string | ||
* The base selector for the variant. | ||
*/ | ||
protected function getBaseItemClass(string $variant): string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need this separate? It's only used once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good, just this documentation change.
date: | ||
type: array | ||
label: 'Date object' | ||
description: 'Date for card. Optional.' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't remember the ticket and it took me a while to remember how to use this. It's not really an object, it's an associative array, let's put some info in here. At least let's write that it uses the same parameters of the bcl date-block template.
No description provided.