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

Update documentation styling #4227

Merged
merged 9 commits into from
Feb 19, 2025
Merged

Update documentation styling #4227

merged 9 commits into from
Feb 19, 2025

Conversation

ghislaineguerin
Copy link
Contributor

@ghislaineguerin ghislaineguerin commented Feb 4, 2025

Updates Documentation Styles to match new marketing site:

NOW:

image image

BEFORE:

image

@ghislaineguerin ghislaineguerin changed the base branch from master to develop February 4, 2025 22:29
@ghislaineguerin ghislaineguerin marked this pull request as ready for review February 5, 2025 11:24
@seancolsen
Copy link
Contributor

I just want to point out that this PR targets develop which means it won't get published until our next release. That's fine with me. But if anyone else wants this to get published sooner, we'll need to cherry-pick these commits on top of master.

@kgodey
Copy link
Contributor

kgodey commented Feb 12, 2025

@ghislaineguerin This looks great! Feedback

  1. Can you please double check the styling of admonitions and code blocks (see old vs. new below)?
    • Please also check on light mode.
  2. I would maybe also consider adding some color to headings.
  3. I would consider differentiating the left sidebar menu and the right sidebar menu a bit more.

Admonition

Screenshot 2025-02-12 at 14-26-52 Relationships - Mathesar Documentation
Screenshot 2025-02-12 at 14-26-39 Relationships - Mathesar Documentation

Code Block

Screenshot 2025-02-12 at 14-27-31 Debugging Mathesar - Mathesar Documentation
Screenshot 2025-02-12 at 14-27-26 Debugging Mathesar - Mathesar Documentation

Copy link
Contributor

@kgodey kgodey left a comment

Choose a reason for hiding this comment

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

See comment.

@kgodey kgodey added this to the v0.2.1 milestone Feb 12, 2025
@kgodey kgodey assigned ghislaineguerin and unassigned kgodey Feb 12, 2025
@kgodey kgodey added the pr-status: revision A PR awaiting follow-up work from its author after review label Feb 12, 2025
@ghislaineguerin
Copy link
Contributor Author

@kgodey I've pushed additional styling changes. I'm not sure if you wanted me to completely change the admonitions or fix anything in particular?

@ghislaineguerin ghislaineguerin added pr-status: review A PR awaiting review and removed pr-status: revision A PR awaiting follow-up work from its author after review labels Feb 17, 2025
@zackkrida
Copy link
Contributor

zackkrida commented Feb 19, 2025

@ghislaineguerin I looked at the code blocks and admonitions and I think they look good 👍

I had a question about font size. I feel like the new fonts are "bigger" at the same font size value as the previous site, and that the new styles look a lot better if the base font size is reduced from .8em to .7em:

Old site (.8em)

Screenshot From 2025-02-19 15-27-17

New site (.8em)

Screenshot From 2025-02-19 15-27-11

New site (.7em)

Screenshot From 2025-02-19 15-27-05

What do you think?

I also think we can probably merge this at this point, once you're replied about the font size.

@zackkrida zackkrida assigned ghislaineguerin and unassigned kgodey Feb 19, 2025
@zackkrida zackkrida added pr-status: revision A PR awaiting follow-up work from its author after review and removed pr-status: review A PR awaiting review labels Feb 19, 2025
@ghislaineguerin
Copy link
Contributor Author

@zackkrida I agree about the font size, I'm ok with making that change and merging it.

@zackkrida zackkrida enabled auto-merge February 19, 2025 22:14
@zackkrida zackkrida added this pull request to the merge queue Feb 19, 2025
Merged via the queue into develop with commit f87be61 Feb 19, 2025
98 checks passed
@zackkrida zackkrida deleted the documentation-style-update branch February 19, 2025 22:18
@zackkrida
Copy link
Contributor

@ghislaineguerin I made the change and merged; thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-status: revision A PR awaiting follow-up work from its author after review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants