Problem/Motivation

In the Olivero theme, the search block form component is not as per the Figma design for small screens
Before Patch
before patch
After Patch
after patch

Steps to reproduce

Make sure you have selected the Olivero theme and open it using Chrome or a similar browser and check for the search block form component

Proposed resolution

After debugging the issue I found that the search block form component is not as mentioned in the Figma design for small screens.

Remaining tasks

None

User interface changes

Adding flex-direction:row-reverse to the following class .block-search-narrow .search-block-form fixes the issue.

API changes

None

Data model changes

None

Release notes snippet

None

CommentFileSizeAuthor
#34 after-search.png147.32 KBsaurav-drupal-dev
#34 before-search.png147.23 KBsaurav-drupal-dev
#28 3367957-24-patch-before.png71.08 KBnilesh.k
#28 3367957-24-patch-after.png77.79 KBnilesh.k
#25 After-patch.png60.42 KBdjsagar
#25 Before-patch.png65.81 KBdjsagar
#24 Screenshot 2023-08-16 at 9.34.19 AM.png123.06 KBgauravvvv
#24 interdiff-16_24.txt1.2 KBgauravvvv
#24 3367957-24.patch2.45 KBgauravvvv
#23 After_patch.png16.52 KBindrapatil
#22 afterpatch.png19.72 KBshiv_sharma
#22 beforepatch.png17.68 KBshiv_sharma
#20 3367957-OK.png98.12 KBde_col
#18 After 3367957-16_0.png52.63 KBfadilraj
#18 Before 3367957-16_0.png52.53 KBfadilraj
#16 3367957-16.patch2.48 KBhamid.ali
#16 after-3367957-16.png82.54 KBhamid.ali
#16 before-3367957-16.png101.83 KBhamid.ali
#12 Figma Design.png58.58 KBfadilraj
#12 After - 3367957-3.png661.79 KBfadilraj
#12 Before -3367957-3.png640.64 KBfadilraj
#3 Search-block-form-component-after.png97.17 KBhamid.ali
#3 Search-block-form-component-before .png100.24 KBhamid.ali
#3 3367957-3.patch904 byteshamid.ali

Issue fork drupal-3367957

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Hamid.ali created an issue. See original summary.

hamid.ali’s picture

hamid.ali commented

I am working on this issue and will provide a patch for the same.

hamid.ali’s picture

hamid.ali commented
Issue summary: View changes
StatusFileSize
new 3367957-3.patch904 bytes
new Search-block-form-component-before .png100.24 KB
new Search-block-form-component-after.png97.17 KB

Have attached the patch to fix this issue.

hamid.ali’s picture

hamid.ali commented
Assigned: hamid.ali » Unassigned
Status: Active » Needs review
Bushra Shaikh’s picture

Bushra Shaikh commented
Assigned: Unassigned » Bushra Shaikh
hamid.ali’s picture

hamid.ali commented
Assigned: Bushra Shaikh » Unassigned
Harish1688’s picture

Harish1688 commented

Hi @Hamid.ali,

can you please share the Olivero theme figma link for mobile view, so i can verified the issue and solution.

smustgrave’s picture

smustgrave commented
Status: Needs review » Postponed (maintainer needs more info)

Figma should be added to the issue summary for others to compare.

hamid.ali’s picture

hamid.ali commented
Assigned: Unassigned » hamid.ali
Status: Postponed (maintainer needs more info) » Active

Here I am attaching the Olivero theme figma link for mobile view mobile view design

hamid.ali’s picture

hamid.ali commented
Assigned: hamid.ali » Unassigned
Status: Active » Needs review
hamid.ali’s picture

hamid.ali commented
Priority: Minor » Normal
fadilraj’s picture

fadilraj commented
StatusFileSize
new Before -3367957-3.png640.64 KB
new After - 3367957-3.png661.79 KB
new Figma Design.png58.58 KB

Applied the patch 3367957-3.patch. Although the position of the search bar has been corrected as per the Figma design, the size of the 'Search' icon does not seem to match. Below are attached screenshots. Changing the status to 'Needs Work' for now.

fadilraj’s picture

fadilraj commented
Status: Needs review » Needs work
lokeshsahu’s picture

lokeshsahu commented

Working on this issue

hamid.ali’s picture

hamid.ali commented
Assigned: Unassigned » hamid.ali
Status: Needs work » Active

I am working on this will provide the updated patch for the same.

hamid.ali’s picture

hamid.ali commented
StatusFileSize
new before-3367957-16.png101.83 KB
new after-3367957-16.png82.54 KB
new 3367957-16.patch2.48 KB

Fixed the search icon as per the Figma attached the updated patch along with the before and after screenshots.
Before Patch
before patch
After Patch
after patch

hamid.ali’s picture

hamid.ali commented
Assigned: hamid.ali » Unassigned
Status: Active » Needs review
fadilraj’s picture

fadilraj commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new Before 3367957-16_0.png52.53 KB
new After 3367957-16_0.png52.63 KB

The patch 3367957-16_0.patch seems to be working fine and does match the Figma design. Screenshots are attached below, for before and after the patch. Changing the status to RTBC.

smustgrave’s picture

smustgrave commented
Status: Reviewed & tested by the community » Needs review
Issue tags: +Needs subsystem maintainer review

Tagging for review by sub maintainer.

Also @Hamid.Ali you shouldn’t assign tickets to yourself unless you’re a maintainer. Per the policy

#14 was actually the correct way to go about it.

de_col’s picture

de_col commented
StatusFileSize
new 3367957-OK.png98.12 KB

Hi,

I confirm that Patch #16 works fine on:
Drupal Core version: 10.1.0-dev
PHP Version: 8.2.6

yovanny.gomez.oyola’s picture

yovanny.gomez.oyola commented

Hello everyone,

I have validated and the patch #16 works well with Drupal: 10.1.0-dev and PHP: 8.1.19

Regards,

shiv_sharma’s picture

shiv_sharma commented
StatusFileSize
new beforepatch.png17.68 KB
new afterpatch.png19.72 KB

As per figma design patch #16 is working fine.
Figma design link for reference.
https://www.figma.com/file/x5zBLbvoW1jsvyAOt4Gp9I/Olivero-Theme---Public...

indrapatil’s picture

indrapatil commented
StatusFileSize
new After_patch.png16.52 KB

I Reviewed patch #16 I attached the screen shot

gauravvvv’s picture

gauravvvv commented
StatusFileSize
new 3367957-24.patch2.45 KB
new interdiff-16_24.txt1.2 KB
new Screenshot 2023-08-16 at 9.34.19 AM.png123.06 KB

Everything looks good except the removing the padding-inline. I have attached the patch and interdiff for same. I have added before and after patch screenshot.

djsagar’s picture

djsagar commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new Before-patch.png65.81 KB
new After-patch.png60.42 KB

I applied patch #24, now form component is as per the Figma design and I attached the screen shot.

smustgrave’s picture

smustgrave commented
Status: Reviewed & tested by the community » Needs review

Still needs review by sub maintainer

smustgrave’s picture

smustgrave commented
Component: CSS » Olivero theme
nilesh.k’s picture

nilesh.k commented
StatusFileSize
new 3367957-24-patch-after.png77.79 KB
new 3367957-24-patch-before.png71.08 KB

Hi

patch #24 has been successfully applied on my local environment. I have also added screenshots, both

before
before
and
after
after

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Was RTBC in #25

Still needs submaintainer sign off but putting back.

Am told design decisions were made before olivero launch that aren't in the figmas.

Personally for me this is a works by design. Magnifying glass on the left seems odd.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 24: 3367957-24.patch, failed testing. View results

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

saurav-drupal-dev made their first commit to this issue’s fork.

saurav-drupal-dev opened merge request !9784

saurav-drupal-dev’s picture

saurav-drupal-dev commented
Status: Needs work » Needs review
StatusFileSize
new before-search.png147.23 KB
new after-search.png147.32 KB

i have fixed the issue for 11.x please review

before

after

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs review » Closed (won't fix)

Just seeing this now. Thank you for all the work, but this was an intentional decision made during the implementation phase (not sure if it's documented though).

Closing as won't fix.

smustgrave closed merge request !9784