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

Redesign banner and mobile menu bar #280

Merged
merged 1 commit into from
Aug 14, 2023

Conversation

jonathanhefner
Copy link
Member

This commit redesigns the banner that was formerly displayed at the top of each page. The banner and the logo are now integrated, and they also serve as the mobile menu bar.

The menu.svg and close.svg files are from Feather icons v4.29.0, and are licensed under the MIT license.

Before After Before (mobile) After (mobile)
before1a after1a before1b after1b
before2a after2a before2b after2b
before3a after3a before3b after3b
before4 after4

@p8
Copy link
Member

p8 commented Aug 8, 2023

Thanks for working on this @jonathanhefner . I think overall this is an improvement.

It seems the margins aren't correct for the mobile page:
https://github.com/rails/sdoc/assets/771968/4c368363-93e3-4f14-921b-62d19e00b422
edit Hmm, this seems broken on main as well.

@jonathanhefner
Copy link
Member Author

It seems the margins aren't correct for the mobile page:
https://github.com/rails/sdoc/assets/771968/4c368363-93e3-4f14-921b-62d19e00b422
edit Hmm, this seems broken on main as well.

Yes, the problem goes back at least as far as when #227 was submitted: https://deploy-preview-227--sdoc.netlify.app/files/actionpack/lib/action_controller/base_rb. (The preview for #226 and earlier have expired, so I can't check those.)

I am working on a PR that updates margins, so I will take care of it. 👍

This commit redesigns the banner that was formerly displayed at the top
of each page.  The banner and the logo are now integrated, and they also
serve as the mobile menu bar.

The `menu.svg` and `close.svg` files are from [Feather icons][feather]
v4.29.0, and are [licensed under the MIT license][license].

[feather]: https://feathericons.com/
[license]: https://github.com/feathericons/feather/blob/v4.29.0/LICENSE
@jonathanhefner jonathanhefner merged commit edb3ec8 into rails:main Aug 14, 2023
jonathanhefner added a commit to jonathanhefner/sdoc that referenced this pull request Aug 20, 2023
In rails#223, we began generating default `<h1>` headings for modules.
However, these headings were only generated for modules that had a
description.  If a module had no comment to begin with, it would not get
a generated heading.

Furthermore, since rails#280, each module's full name has been prominently
displayed at the top of its page.  These names are visually redundant
with generated headings, which also use the full name.

To unify the design and ensure that all modules have an `<h1>`, this
commit converts each module's full name into an `<h1>` heading inside an
`<hgroup>`, and uses postprocessing to pull any comment-based `<h1>`
into the `<hgroup>`.

This commit also renames the "Included Modules" section to "Inherits
From", and moves any listed base class from the top of the page to the
top of that section.  This change focuses the `<h1>`, both visually and
from an SEO perspective.
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