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.

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

FIX: Fix search icons displayed outside with small base font size #1278

Merged
merged 3 commits into from Apr 13, 2023

Conversation

Maetveis
Copy link
Contributor

@Maetveis Maetveis commented Apr 1, 2023

When --pst-font-size-base is small (i.e. 12px) the search icons (magnifying-glass and keyboard shortcut hint) could appear outside the search bar because their positions did not take the negative margins of their parent container into account.

Use css calc() to include it now.

TBH I don't know why a padding + negative margin is employed on the form element. Online I found that this technique can be used to make just one element (like a header) not constrained by the padding, but here there is only one child element (the <form>). The commit history doesn't help either since the stylesheet had this since it's first version.

Before (300% zoom + --pst-font-size-base: 0.33rem to exaggerate the problem):
before

After:
after

When `--pst-font-size-base` is small (i.e. 12px) the search icons
(magnifying-glass and keyboard shortcut hint) could appear outside the
search bar because their positions did not take the negative margins
of their parent container into account.

Use css `calc()` to include it now.

TBH I don't know why a padding + negative margin is employed on the
form element. Online I found that this technique can be used to make
just one element (like a header) not constrained by the padding,
but here there is only one child element (the `<form>`).
The commit history doesn't help either the stylesheet had this since
it's first version.
@drammock
Copy link
Collaborator

drammock commented Apr 4, 2023

I also have no idea why the container has padding + negative margin. The resultant CSS is... complicated to say the least. Are you willing to try to simplify this bit of CSS (i.e., get rid of the negative margins and adjust what's left accordingly), or are you happy to just have it looking good again?

@Maetveis
Copy link
Contributor Author

Maetveis commented Apr 12, 2023

Are you willing to try to simplify this bit of CSS (i.e., get rid of the negative margins and adjust what's left accordingly), or are you happy to just have it looking good again?

Sure I have checked it out before that it would work, just didn't know if there was any reason why it was the way it was. See latest commit

@drammock drammock mentioned this pull request Apr 12, 2023
@drammock
Copy link
Collaborator

@Maetveis you'll need to merge in current main to get the CIs to pass. I tried doing it for you by pushing to your branch but got permission denied.

@Maetveis
Copy link
Contributor Author

@Maetveis you'll need to merge in current main to get the CIs to pass. I tried doing it for you by pushing to your branch but got permission denied.

I don't remember disabling the allow pushes option on the PR, anyway done now, thanks.

@drammock drammock merged commit fce5e85 into pydata:main Apr 13, 2023
16 checks passed
@drammock
Copy link
Collaborator

I don't remember disabling the allow pushes option on the PR

in my experience sometimes it just doesn't work for unknown reasons. Thanks for the fix!

@Maetveis Maetveis deleted the fix-search-icon-scaling branch April 13, 2023 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants