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.

Sign up for GitHub

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

New TPR service list component with search #494

Merged
merged 23 commits into from
Mar 21, 2023
Merged

New TPR service list component with search #494

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
fca119e
Added twigs for list search and included new teaser view mode
dragos-dumi-ibm Nov 4, 2022
5811278
Added initial arrow link style for search teaser result card
dragos-dumi-ibm Nov 7, 2022
ce7b8a1
Added basic style for cards service list search
dragos-dumi-ibm Nov 7, 2022
278dfd7
added tags to service twig
Nov 7, 2022
be1c80f
removed unique channels var
Nov 7, 2022
e083bda
Stles for service search list
dragos-dumi-ibm Nov 8, 2022
2fed651
Merge branch 'service_list_search' of github.com:dragos-dumi-ibm/drup…
dragos-dumi-ibm Nov 8, 2022
04fe883
Merge branch 'main' into service_list_search
Nov 10, 2022
d1e2216
Merge pull request #2 from dragos-dumi-ibm/service_list_search
dragos-dumi-ibm Nov 10, 2022
694d6ca
Service list search design
oviag Nov 14, 2022
70a7bf8
PLAENH-258 - handle case when tpr search list has 1 page only
dragos-dumi-ibm Nov 16, 2022
b5b45cc
Merge branch 'City-of-Helsinki:main' into main
dragos-dumi-ibm Nov 25, 2022
03ed869
Merge branch 'City-of-Helsinki:main' into main
dragos-dumi-ibm Dec 5, 2022
79c3c7b
Merge branch 'main' of github.com:City-of-Helsinki/drupal-hdbt
dragos-dumi-ibm Jan 4, 2023
aec21d3
Changes for TPR service search after review
dragos-dumi-ibm Jan 5, 2023
3a91473
Refactored existing service list view to pass stylelint and comments …
dragos-dumi-ibm Jan 9, 2023
1dd0c9a
modify icon translation and fix for mobile
oviag Feb 7, 2023
7e2de81
added use statement
oviag Feb 7, 2023
0d5b96c
Merge branch 'main' into main
dragos-dumi-ibm Feb 7, 2023
4c3c6cf
fixed arrow motion to use transition transform instead of transition all
oviag Feb 7, 2023
9634352
Merge branch 'main' of github.com:dragos-dumi-ibm/drupal-hdbt
oviag Feb 7, 2023
3422de4
Merge branch 'main' of github.com:City-of-Helsinki/drupal-hdbt
oviag Mar 1, 2023
01168c5
Merge remote-tracking branch 'helsinki_remote/main'
oviag Mar 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 40 additions & 5 deletions hdbt.theme
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use Drupal\menu_link_content\Plugin\Menu\MenuLinkContent;
use Drupal\node\NodeInterface;
use Drupal\responsive_image\Entity\ResponsiveImageStyle;
use Drupal\views\ViewExecutable;
use Drupal\helfi_tpr\Entity\Service;

/**
* Implements hook_preprocess().
Expand Down Expand Up @@ -822,6 +823,22 @@ function hdbt_preprocess_paragraph(array &$variables) {
}, $paragraph->field_service_list_services->getValue()));
}

if ($paragraph_type == 'service_list_search') {
$ids = '';
$service_ids = '';
if ($paragraph->hasField('field_service_list_services')) {
$ids = implode(',', array_map(function ($service) {
return $service['target_id'];
}, $paragraph->field_service_list_services->getValue()));
}
if ($paragraph->hasField('field_service_list_service_ids')) {
$service_ids = implode(',', array_map(function ($service) {
return $service['value'];
}, $paragraph->field_service_list_service_ids->getValue()));
}
$variables['services_list_views_argument'] = $ids . '|' . $service_ids;
}

// Contact card paragraph.
if (
$paragraph_type == 'contact_card' &&
Expand Down Expand Up @@ -1222,11 +1239,14 @@ function _hdbt_craft_token(string $name, array &$variables) {
* Implements hook_theme_suggestions_HOOK_alter().
*/
function hdbt_theme_suggestions_views_view_alter(array &$suggestions, array $variables) {
if (
isset($variables['view']) &&
!empty($variables['view']->id())
) {
$suggestions[] = 'views_view__' . $variables['view']->id();
if (isset($variables['view'])) {
$view = $variables['view'];
if (!empty($variables['view']->id())) {
$suggestions[] = 'views_view__' . $variables['view']->id();
}
if (!empty($view->getDisplay()->display['id'])) {
$suggestions[] = 'views_view__' . $variables['view']->id() . '__' . $variables['view']->current_display;
}
}
}

Expand Down Expand Up @@ -1276,6 +1296,9 @@ function hdbt_preprocess_views_view(&$variables) {
if ($variables['view']->total_rows != NULL) {
$variables['total_rows'] = $variables['view']->total_rows;
}
if ($variables['view']->id() == 'service_list') {
$variables['is_ajax_request'] = \Drupal::request()->isXmlHttpRequest();
}
}

/**
Expand Down Expand Up @@ -1321,6 +1344,18 @@ function hdbt_preprocess_tpr_service_channel(&$variables) {
}
}

/**
* Implements hook_preprocess_HOOK().
*/
function hdbt_preprocess_tpr_service(&$variables) {
if (!isset($variables['entity']) || !$variables['entity'] instanceof Service) {
return;
}
$entity = $variables['entity'];

$variables['service_url'] = !$entity->isNew() ? $entity->toUrl('canonical') : NULL;
}

/**
* Implements hook_preprocess_HOOK().
*/
Expand Down
111 changes: 53 additions & 58 deletions src/scss/06_components/pages/_service.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,85 +24,80 @@ $teaser-icon-size-desktop: 48px;
margin-right: calc(#{$spacing-and-half} / 2);
}

.service__link {
border: 1px solid $color-black;
display: block;
height: 100%;
margin-top: $spacing-half;
padding: $spacing-and-half calc(#{$spacing-and-half} + #{$teaser-icon-size-mobile} + #{$spacing}) $spacing-and-half $spacing-and-half;
position: relative;
text-decoration: none;

@include breakpoint($breakpoint-l) {
display: flex;
flex-direction: column;
justify-content: space-between;
line-height: 0; // Remove extra space under the arrow icon on larger card
margin-top: 0;
min-height: 240px; // As in designs
padding: $spacing-double $spacing-and-half calc(#{$spacing-and-half} + #{$spacing-and-half});
}

&::before {
background-color: $color-gold;
content: '';
display: block;
height: 100%;
left: 0;
position: absolute;
top: 0;
width: $spacing-half;

@include breakpoint($breakpoint-l) {
bottom: 0;
height: $spacing-and-half;
right: 0;
top: auto;
width: 100%;
}
}

&:hover {
text-decoration: underline;

.hel-icon {
margin-left: $spacing;

@media (prefers-reduced-motion) {
margin-left: 0;
}
}
}
}

.service__title {
@include font('h4');
margin: 0;
}

.hel-icon {
--icon-size: #{$teaser-icon-size-mobile};
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was the icon size for mobile removed here? We should not break existing card layouts.

Copy link
Contributor

Choose a reason for hiding this comment

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

Have you checked that you did not break mobile layouts for these? To me it seems that you broke the arrows from these cards. See the prod and your branch difference.
image
image

bottom: 50%;
left: auto;
position: absolute;
right: $spacing;
transform: translateY(50%);
dragos-dumi-ibm marked this conversation as resolved.
Show resolved Hide resolved
transition: all 0.3s;

@media (prefers-reduced-motion) {
transition: none;
}
@include pseudo-icon('arrow-right');
@include hover-link-arrow-transition;

@include breakpoint($breakpoint-l) {
--icon-size: #{ $teaser-icon-size-desktop};
margin-top: $spacing-and-half;
position: static;
right: auto;
dragos-dumi-ibm marked this conversation as resolved.
Show resolved Hide resolved
transform: none;
}
}
}

.service__link {
border: 1px solid $color-black;
display: block;
height: 100%;
margin-top: $spacing-half;
padding: $spacing-and-half calc(#{$spacing-and-half} + #{$teaser-icon-size-mobile} + #{$spacing}) $spacing-and-half $spacing-and-half;
position: relative;
text-decoration: none;

@include breakpoint($breakpoint-l) {
display: flex;
flex-direction: column;
justify-content: space-between;
line-height: 0; // Remove extra space under the arrow icon on larger card
margin-top: 0;
min-height: 240px; // As in designs
padding: $spacing-double $spacing-and-half calc(#{$spacing-and-half} + #{$spacing-and-half});
}

&::before {
background-color: $color-gold;
content: '';
display: block;
height: 100%;
left: 0;
position: absolute;
top: 0;
width: $spacing-half;

@include breakpoint($breakpoint-l) {
bottom: 0;
height: $spacing-and-half;
right: 0;
top: auto;
width: 100%;
}
}

&:hover {
text-decoration: underline;

.hel-icon {
transform: translateX($spacing-quarter);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this animation size adjusted? Now it seems too small compared to the icon size.

Please do not move code and change code in same commit. Also, please make smaller commits that only change one thing and have a properly formatted message so that we can follow your logic.

By properly formatted message I mean that each commit message should read as if it had this string in front of it:
"If accepted, this commit will: "

Like this:
"Reduce service-link icon animation amount."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello. Will check again the comments and the mobile version.
Mainly the changes you've mentioned in this review of why were done is what I've mentioned in the comment above

Regarding style lint and class related comments, I have also corrected the existing twig and scss for this existing component, to pass the stylelint - https://www.hel.fi/en/decision-making/contact-the-city-of-helsinki/advisory-services One comment here: the animation in the new card.twig is different than current one at https://www.hel.fi/en/decision-making/contact-the-city-of-helsinki/advisory-services I assume is ok to refactor it to use the same new animation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we've updated the icon for existing card. the transition all was an issue for style lint as well as other styles around that arrow so that's why we removed some styles initially. Now we've refactored the code around that arrow to keep same animation but to pass stylelint checks


@media (prefers-reduced-motion) {
transform: none;
}
}
}
}

.service--units__container {
margin-bottom: $spacing-quadruple;
margin-top: $spacing-quadruple;
Expand Down
98 changes: 66 additions & 32 deletions src/scss/06_components/paragraphs/_service-list.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,44 +8,54 @@
}

.views-exposed-form {
border-bottom: 1px solid $color-black-10;
Arkkimaagi marked this conversation as resolved.
Show resolved Hide resolved
padding-bottom: $spacing-triple;

@include breakpoint($breakpoint-l) {
padding-bottom: $spacing-quadruple;
}

@include breakpoint($breakpoint-m) {
align-items: flex-end;
display: flex;
}
}

.form-item {
@include breakpoint($breakpoint-m) {
margin-bottom: 0;
width: 70%;
}
.form-item {
@include breakpoint($breakpoint-m) {
margin-bottom: 0;
width: 70%;
}
}

.hds-text-input__input-wrapper {
&::after {
@include pseudo-icon('search', 20px, $color-black, block);
position: absolute;
right: $spacing;
top: 50%;
transform: translateY(-50%);
}

.hds-text-input__input {
border-radius: 0;
padding-right: $spacing-triple;
}
.hds-text-input__input-wrapper {
&::after {
@include pseudo-icon('search', 20px, $color-black, block);
position: absolute;
right: $spacing;
top: 50%;
transform: translateY(-50%);
}

.form-actions {
@include breakpoint($breakpoint-m) {
display: flex;
width: 30%;
}
.hds-text-input__input {
border-radius: 0;
padding-right: $spacing-triple;
}
}

.form-actions {
@include breakpoint($breakpoint-m) {
display: flex;
width: 30%;
}
}

.service-list__count-container {
@include font('lead');
display: block;
margin-bottom: $spacing-double;
margin-top: $spacing-triple;

@include font('lead');
width: 100%;

.service-list__count {
Expand All @@ -63,16 +73,40 @@
margin-right: calc(-#{$spacing-and-half} / 2);
margin-top: $spacing-triple;
}
}
}

.views--service-list--block-search {
.services-search__results {
> .views-row + .views-row {
margin-top: $spacing;
}
}
}

.views-row {
@include breakpoint($breakpoint-l) {
margin-top: $spacing-and-half;
width: 25%;
}
.views--service-list--block {
.views-row {
@include breakpoint($breakpoint-l) {
margin-top: $spacing-and-half;
width: 25%;
}

&:nth-child(-n+4) {
margin-top: 0;
}
&:nth-child(-n+4) {
margin-top: 0;
}
}
}

.component--service-list-search {
background-color: $color-silver-light;

.component__container {
padding-bottom: $spacing-double;
padding-top: $spacing-double;

@include breakpoint($breakpoint-m) {
padding-bottom: $spacing-quadruple;
padding-top: $spacing-quadruple;
}
}
}
8 changes: 8 additions & 0 deletions templates/module/helfi_tpr/tpr-service--teaser-search-result.html.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{% embed '@hdbt/component/card.twig' with {
card_modifier_class: 'card--tpr-service-teaser',
card_title: entity.label,
card_url: service_url,
card_description: content.description,
card_tags: content.errand_services[0]['#items'],
} %}
{% endembed %}
15 changes: 15 additions & 0 deletions templates/paragraphs/paragraph--service-list-search.html.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{% block paragraph %}
{% embed "@hdbt/misc/component.twig" with
{
component_classes: [ 'component--service-list-search' ],
component_title: content.field_service_list_title,
component_description: content.field_service_list_description,
}
%}

{% block component_content %}
{{ drupal_view('service_list', 'block_search', services_list_views_argument) }}

{% endblock component_content %}
{% endembed %}
{% endblock paragraph %}
Loading