Skip to content

Make search tab style match navigation #587

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 6 commits into from
Feb 8, 2020
Merged

Conversation

Zexbe
Copy link
Contributor

@Zexbe Zexbe commented Feb 3, 2020

  1. Change the icon to use font awesome
  2. Use the same border the other menu items are using

small

medium

large

@GuillaumeGomez
Copy link
Member

I guess you tried to use "fa-search" from font-awesome. Unfortunately, it doesn't work unless you add "font-family: FontAwesome" on the element. But then, the text would change as well. So not really ideal. :)

The border change looks good.

@Zexbe
Copy link
Contributor Author

Zexbe commented Feb 3, 2020

Oh no, I solved that issue. I lost the change somehow, one second.

@GuillaumeGomez
Copy link
Member

Please add a screenshot too so people can see the changes.

@Zexbe
Copy link
Contributor Author

Zexbe commented Feb 3, 2020

Okay, I added pictures for three different media sizes.

@GuillaumeGomez
Copy link
Member

Thanks! I'll merge once CI pass.

@pietroalbini
Copy link
Member

Could you move the icon out of the placeholder so it's shown even when something is written in the input?

@Zexbe
Copy link
Contributor Author

Zexbe commented Feb 3, 2020

I'm not sure how I would make that work.
The icon would have to be all the way to the left, or right to keep it from moving as you typed. It would also have to become a button, because it would look clickable if it stayed there.

@pietroalbini
Copy link
Member

It'd be fine for me if the icon is all the way to the left.

@pietroalbini
Copy link
Member

...and if we do that we could also left-align the input to make it less weird.

@Zexbe
Copy link
Contributor Author

Zexbe commented Feb 3, 2020

It is done, it looks even better now. I updated the pictures

@jyn514
Copy link
Member

jyn514 commented Feb 4, 2020

Any objections to merging?

@GuillaumeGomez
Copy link
Member

Not from me. Let's wait for @pietroalbini's approval. ;)

@pietroalbini
Copy link
Member

On the label add:

label {
  cursor: pointer;
  padding-left: 0.5rem;
  font-size: 0.8em;
}

That way is more closely aligned with the rest of the items:

2020-02-04--18-53-05
2020-02-04--18-52-38

@Zexbe
Copy link
Contributor Author

Zexbe commented Feb 6, 2020

Okay, updated the search label

@Zexbe
Copy link
Contributor Author

Zexbe commented Feb 8, 2020

r? @pietroalbini

@pietroalbini pietroalbini merged commit 2241e06 into rust-lang:master Feb 8, 2020
@pietroalbini
Copy link
Member

Thanks!

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.

4 participants