Related to #3125328: Search form submit button should only be altered if it's in the header

When the search block form is moved outside of the Primary Menu region, it is not themed properly. It still inherit's its markup from the preprocess function, but it is not styled.

We could either

1) Style it
2) Have preprocess only modify it if it's in the primary menu region.

CommentFileSizeAuthor
#27 interdiff-20-27.txt362 bytesrahulrasgon
#27 3139046-27.patch1.27 KBrahulrasgon
#23 search-block.png220.55 KBsd9121
#20 3139046-20.patch1.35 KBhansa11
#12 patch10-applied.png176.38 KBmatija5
#10 other_region_search.png348.2 KBkiran.kadam911
#10 secondary_region_search_wide.png348.82 KBkiran.kadam911
#10 primary_region_search_narrow.png321.21 KBkiran.kadam911
#10 search-block-not-styled-3139046-10.patch1.44 KBkiran.kadam911
#7 3139046-7.patch3.41 KBmatija5
#3 after-patch-reponsive.png27.36 KBkomalk
#3 before-patch.png163.63 KBkomalk
#3 after-patch.png73.49 KBkomalk
#3 3139046-3.patch2.25 KBkomalk
Lullabot_Daily_Report___Daily_Report.png37.67 KBmherchel

Comments

mherchel created an issue. See original summary.

komalk’s picture

komalk commented
Assigned: Unassigned » komalk
komalk’s picture

komalk commented
Status: Active » Needs review
StatusFileSize
new 3139046-3.patch2.25 KB
new after-patch.png73.49 KB
new before-patch.png163.63 KB
new after-patch-reponsive.png27.36 KB
komalk’s picture

komalk commented
Related issues: +#3142850: Adapt button height to the input element height

The button height to the input element is disturbed refer the after-patch screen shot .
tagging the related issue for the button height.

komalk’s picture

komalk commented
Assigned: komalk » Unassigned
matija5’s picture

matija5 commented

Search form in menu has magnifier icon for submit button. Wouldn't be better to have it also in sidebar instead of text?

Icon is not showing because its styling is applied for .search-narrow__wrapper and .search-wide__wrapper classes. I think search should have some global styling and then its overrides in header search files.

matija5’s picture

matija5 commented
StatusFileSize
new 3139046-7.patch3.41 KB

Hi, after applying patch #3 the search button is displayed.

But I also did my take on this. I moved search icon to new css file for search and left specific styling for icon in header search files.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs review » Needs work

I would really like to address this within pre-process as opposed to CSS. The /templates/form/input--submit--header-search.html.twig template gets called on all search blocks. I'd like the template to only be called if the block is located in the primary nav or secondary nav regions.

I believe we did something similar for the menu region at https://git.drupalcode.org/project/olivero/-/blob/8.x-1.x-dev/olivero.th...

kiran.kadam911’s picture

kiran.kadam911
he/him
Location Mumbai, India 🇮🇳
commented
Assigned: Unassigned » kiran.kadam911

taking this up as per the mentioned approach in last comment by @mherchel.

kiran.kadam911’s picture

kiran.kadam911
he/him
Location Mumbai, India 🇮🇳
commented
Assigned: kiran.kadam911 » Unassigned
Status: Needs work » Needs review
StatusFileSize
new search-block-not-styled-3139046-10.patch1.44 KB
new primary_region_search_narrow.png321.21 KB
new secondary_region_search_wide.png348.82 KB
new other_region_search.png348.2 KB

Sorry for the delay, Kindly review the attached patch.

Attaching screenshots of search blocks if it's in the menu region(primary and secondary) and if it's in another region.

Search block in the primary and secondary menu region:

Search block in another region:

Thanks!

Lal_’s picture

Lal_
He\Him
commented 
+++ b/olivero.theme
@@ -170,6 +170,19 @@ function olivero_form_alter(&$form, FormStateInterface $form_state, $form_id) {
+  $block_ids = ['olivero_search_form_wide', 'olivero_search_form_narrow'];
+  foreach ($block_ids as $block_id) {
+    $block = \Drupal::entityTypeManager()
+      ->getStorage('block')
+      ->load($block_id);
+    if ($block) {
+      $region = $block->getRegion();
+      $form['actions']['submit']['#attributes']['region'] = $region;
+    }

Can we incorporate this under a condition... We don't want this run every time when form alter triggers

matija5’s picture

matija5 commented
StatusFileSize
new patch10-applied.png176.38 KB

hi, thanks for the patch. After applying patch#10 I don't have Search word in button. It is also applying template 'themes/contrib/olivero/templates/form/input--submit--header-search.html.twig' for search forms outside header.

kiran.kadam911’s picture

kiran.kadam911
he/him
Location Mumbai, India 🇮🇳
commented
Assigned: Unassigned » kiran.kadam911
Status: Needs review » Needs work
kiran.kadam911’s picture

kiran.kadam911
he/him
Location Mumbai, India 🇮🇳
commented
Assigned: kiran.kadam911 » Unassigned
sonvir249’s picture

sonvir249 commented
Assigned: Unassigned » sonvir249
matija5’s picture

matija5 commented

Hi @sonvir249, can I give this a try?

Lal_’s picture

Lal_
He\Him
commented
Assigned: sonvir249 » Unassigned
hansa11’s picture

hansa11 commented
Assigned: Unassigned » hansa11
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented

@hansa11 Any progress on this? I also see that you have assigned two other issues to yourself.

hansa11’s picture

hansa11 commented
Assigned: hansa11 » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 3139046-20.patch1.35 KB

@mherchel, yes I've been working on it but I was stuck at one point on this, now it's done and here is the patch.

Please review.

Thanks!

sd9121’s picture

sd9121 commented
Assigned: Unassigned » sd9121
sd9121’s picture

sd9121 commented
Assigned: sd9121 » Unassigned
Status: Needs review » Reviewed & tested by the community

@hansa11,

I have reviewed your patch, it looks good to me.

Screenshot for reference:

search-block

Thanks!

sd9121’s picture

sd9121 commented
StatusFileSize
new search-block.png220.55 KB
rahulrasgon’s picture

rahulrasgon commented

RTBC + 1 for Patch #20

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/olivero.theme
@@ -171,7 +173,7 @@ function olivero_form_alter(&$form, FormStateInterface $form_state, $form_id) {
+      $form['actions']['submit']['#olivero_is_header_search_submit'] = FALSE;

Correct me if I'm wrong, but we don't need to set this to FALSE. We can just remove this line, correct?

rahulrasgon’s picture

rahulrasgon commented
Assigned: Unassigned » rahulrasgon
rahulrasgon’s picture

rahulrasgon commented
Assigned: rahulrasgon » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 3139046-27.patch1.27 KB
new interdiff-20-27.txt362 bytes

Please review the patch.
Thanks

sd9121’s picture

sd9121 commented
Assigned: Unassigned » sd9121
sd9121’s picture

sd9121 commented
Assigned: sd9121 » Unassigned
Status: Needs review » Reviewed & tested by the community

@rahulrasgon,

Your patch looks good to me.

Thanks!

  • mherchel committed 17a2242 on 8.x-1.x authored by hansa11 
    Issue #3139046 by kiran.kadam911, komalkolekar, vebrovski, rahulrasgon,...
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Reviewed & tested by the community » Fixed

Committed. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.