Skip to content

Fix: Fixing search bar in responsive screens. #359

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

Merged
merged 10 commits into from
Mar 1, 2024

Conversation

DhairyaMajmudar
Copy link
Member

@DhairyaMajmudar DhairyaMajmudar commented Feb 25, 2024

GitHub Issue: #345 #355 #374
Fixes #345
Fixes #355
Fixes #374

Summary: fixed the search bar in responsive mode.

Changes Done:

  1. Reduced width of the div.
  2. Made div display inline in responsive mode.
  3. Reduced the gap between search icon and text
  4. Added the shadow for fixing issue 374
  5. incorporated changes for the issue 355

Files Changed

  1. components/Layout.tsx
  2. pages/index.page.tsx
  3. styles/globals.css

Screenshots

Before
Screenshot from 2024-02-25 19-15-46

After
image

Do you think resolving this issue might require an Architectural Decision Record (ADR)? (significant or noteworthy)

No

@DhairyaMajmudar
Copy link
Member Author

@benjagm pls. have a look : )

@benjagm
Copy link
Collaborator

benjagm commented Feb 26, 2024

Nice job @DhairyaMajmudar !

It worked for all cases but one: Ipad Mini resolutions:

Screenshot 2024-02-26 at 15 19 55

Copy link
Collaborator

@benjagm benjagm left a comment

Choose a reason for hiding this comment

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

Nice job @DhairyaMajmudar !

It worked for all cases but one: Ipad Mini resolutions:

@DhairyaMajmudar
Copy link
Member Author

Thanks for testing , I will improve the iPad one and update my pr 😃

@DhairyaMajmudar
Copy link
Member Author

@benjagm I have made the changes for iPad mini and also incorporated the changes from PR #358.

@benjagm
Copy link
Collaborator

benjagm commented Feb 26, 2024

You did amazing here incorporating all the request @DhairyaMajmudar. Congrats!!!

Now that we are fixing this section of the landing let's make the last change in the seach button to make the the lens icon appear closer to the Search text so we leave this PERFECT. Please see the image below.

Screenshot 2024-02-26 at 20 31 03

What do you think about making this last change?

@DhairyaMajmudar
Copy link
Member Author

Thank you for your appreciation @benjagm : )

I think the changes you are suggesting are good making icons and text close to each other is a good idea 👍🏻

@benjagm
Copy link
Collaborator

benjagm commented Feb 26, 2024

@Mvishal123 I am adding you as reviewer of this PR considering we are adding some of your changes for #355

@benjagm benjagm mentioned this pull request Feb 26, 2024
@DhairyaMajmudar
Copy link
Member Author

@benjagm I have made the suggested changes.

image

@benjagm
Copy link
Collaborator

benjagm commented Feb 27, 2024

I have checked the last version of code and I found something in the search button.
It seems you are centering the button content just considering the text, however what need to be centered is the block compound by the lens icon + text. See image:

Screenshot 2024-02-27 at 18 36 36

@DhairyaMajmudar
Copy link
Member Author

Sure will look into this 👍🏻

@DhairyaMajmudar
Copy link
Member Author

@benjagm I have updated my PR with the suggested changes

image

@benjagm
Copy link
Collaborator

benjagm commented Feb 28, 2024

@benjagm I have updated my PR with the suggested changes

I was speaking about the blue button @DhairyaMajmudar !! But great you fixed that one too.
So the only 2 thing pending are:

  1. fix the centering of the content of the blue button
  2. I just found this when resizing the window. Can we have the 3 buttons the same width in all scenarions?
Screenshot 2024-02-28 at 20 10 45

Copy link
Collaborator

@benjagm benjagm left a comment

Choose a reason for hiding this comment

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

Left 2 small comments in the PR discussion:
#359 (comment)

@DhairyaMajmudar
Copy link
Member Author

Left 2 small comments in the PR discussion: #359 (comment)

Thankyou will fix this 👍🏻

@benjagm benjagm added the Status: In Progress This issue is being worked on, and has someone assigned. label Feb 29, 2024
@DhairyaMajmudar
Copy link
Member Author

@benjagm I have done the suggested fixes pls. have a look

Copy link
Collaborator

@benjagm benjagm left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks!!

@benjagm benjagm merged commit 6d1e3fc into json-schema-org:main Mar 1, 2024
@DhairyaMajmudar DhairyaMajmudar deleted the search branch March 1, 2024 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: In Progress This issue is being worked on, and has someone assigned.
Projects
None yet
2 participants