Skip to content

Fix: Search box doesn't fit in overlays on mobile using MediaQuery Tag #2434

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

Closed
wants to merge 7 commits into from

Conversation

KSSaiTeja
Copy link

@KSSaiTeja KSSaiTeja commented Sep 13, 2023

Fixes: #2369

Changes:

I have used <MediaQuery> tag to fix the issue

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number, i.e. Fixes #123

@KSSaiTeja
Copy link
Author

@lindapaiste please review my PR.

Copy link
Collaborator

@lindapaiste lindapaiste left a comment

Choose a reason for hiding this comment

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

Please run this command and push the changes.

git revert 8c8dfa16c4ad9ddb7d9522ee9add70de9d5d6363

That will remove the unrelated changes to CollectionListRow.

When you start a new branch for a PR you want to start from upstream/develop and not include the code from your other branches. I know this git stuff can be a real headache.

</header>
<MediaQuery query="(max-width: 767px)">
{actions && (
<div className="overlay__actions-mobile">{actions}</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's a some left padding to this new CSS class. This will align it with the QuickAddWrapper.

.overlay__actions-mobile {
  padding-left: #{24/ $base-font-size}rem;
}

(I know I said in the issue not to worry about padding, but seeing it in action...I've changed my mind 😝)

image

Copy link
Author

Choose a reason for hiding this comment

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

so do you want me run git revert 8c8dfa16c4ad9ddb7d9522ee9add70de9d5d6363 and push again..?

Copy link
Author

Choose a reason for hiding this comment

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

or else do i need to do any changes?

@KSSaiTeja
Copy link
Author

@lindapaiste hope this will not bother you.. i reverted other branch commit for this.. pushed again. please review it in your freetime and let me know if any other changes required

@raclim
Copy link
Collaborator

raclim commented May 30, 2024

Thanks for your work on this! Due to the amount of time that's passed since this pull request was opened, I'm going to close this for now. I'm sorry that we ended up not being able to merge this in, but please feel free to reopen a new pull request for this issue!

@raclim raclim closed this May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Search box doesn't fit in overlays on mobile
3 participants