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

Improve docs header style #2516

Merged
merged 3 commits into from
Jun 11, 2024
Merged

Improve docs header style #2516

merged 3 commits into from
Jun 11, 2024

Conversation

FireIsGood
Copy link
Contributor

Sanity check:

  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

The current website style for the header is a bit jank and leaves a bit to be desired. The search bar and navigation are both left aligned which makes it feel a bit cluttered, though that's not too bad of an issue. The real issue I had with it was how the mobile styles looked. The spacing method causes the links to be shifted left a bit and the search bar to sit right against the text.

image

Before

Desktop styles

Home Docs
home docs

Mobile styles

Home Docs
home mobile docs mobile

After

Desktop styles

Home Docs
home docs

Mobile styles

Home Docs
home mobile docs mobile

Additional Information

A lot of the jank comes from how the nav was previously just using inline-block and margin-right based styles.

Also, there was a random .container style applied to only the home page's header which was reused on the contents of the body as well which made restyling a pain. I have made the decision to remove it from the header section entirely for consistency between the home and docs pages.

There are also a lot of other evil styles such as the .selling-point styles in general, though that would be more of a style rewrite which is more opinionated and likely better saved for a full website rewrite.

causes a lot of lag and also probably isn't the best to inline since we
have scss preprocessing
Copy link
Collaborator

@Keats Keats left a comment

Choose a reason for hiding this comment

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

LGTM except the quote change

<script type="text/javascript" src="{{ get_url(path="elasticlunr.min.js") }}"></script>
<script type="text/javascript" src="{{ get_url(path="search.js") }}"></script>
<script type="text/javascript" src="{{ get_url(path='elasticlunr.min.js') }}"></script>
<script type="text/javascript" src="{{ get_url(path='search.js') }}"></script>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I prefer " to ', any reasons to change those?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the editor lsp or formatter is set to regular HTML, it can get hung up on the fact that there are double quotes inside the strings and refuses to work. My reason for this case was that in JS you can use single quotes in double quotes and vice versa, so they should likely be the same in HTML.

I believe the specific reason in this case was that I was using Prettier which was angry that the tags weren't closed due to some weirdness there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

good point i didn't think of that

@Keats Keats merged commit 606c2be into getzola:master Jun 11, 2024
7 checks passed
@FireIsGood FireIsGood deleted the improve-styles branch June 12, 2024 19:30
berdandy pushed a commit to berdandy/azola that referenced this pull request Sep 17, 2024
* refactor: pattern to separate file

causes a lot of lag and also probably isn't the best to inline since we
have scss preprocessing

* fix: escape quotes properly

* feat: improve header styles
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.

2 participants