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

Rearrange sections to use basic-ng names #662

Merged
merged 6 commits into from May 16, 2022

Conversation

choldgraf
Copy link
Collaborator

@choldgraf choldgraf commented May 12, 2022

This rearranges our template structure in order to align with the basic-ng theme structure. It roughly follows this structure (from the basic-ng example build)

image

It should not change any user-facing things. Here are the main changes:

  • The layout.html now broken up into several parts in sections/
  • Added classes to each section container (e.g. bd-header-primary and bd-header-primary__container)
  • Cleaned up some CSS that needed fixing due to the new structure
  • Moved all mini HTML templates into a components folder instead of _templates (so it's clearer what kinds of things go there)

I think there is still more we could do (particularly, cleaning up and organizing the components folder because it is a bit messy now). But I think we could do that in future PRs.

last part of #610

Follow-ups that we could try after this PR (for future ref):

  • Clean up components/ so that it is broken into section-specific areas (e.g. nav/icon-links.html instead of nav-icon-links.html).
  • Rework our CSS classes to use the new section areas, and deprecate the old ones (e.g. bd-sidebar-primary and deprecate bd-sidebar).
  • Potentially overhaul the way that the header items are configured so that we just configure them with python, instead of a combination of python + lists of HTML templates.

@choldgraf
Copy link
Collaborator Author

cc @damianavila and @12rambau since you have been helping a lot so far :-) it would be great if we could get this merged relatively quickly because it will become out of date soon

Copy link
Collaborator

@12rambau 12rambau left a comment

Choose a reason for hiding this comment

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

I know very few things about basic-ng but the documentation build nicely and I didn't spot any buggy section. In general, the tidying work is super good it's way easier to navigate in the files and elements.
I don't know if it was already the case before but the use of the BEM synthax is much appreciated!

I think this new classes will be very helpfull for #641


{% block docs_navbar %}
<nav class="navbar navbar-light navbar-expand-lg bg-light fixed-top bd-navbar" id="navbar-main">
{%- include "docs-navbar.html" %}
<nav class="bd-header-primary navbar navbar-light navbar-expand-lg bg-light fixed-top bd-navbar" id="navbar-main">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this class bd-header-primary and not bd-header as the section name and the image you shared in the PR ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good catch! the latest push renames this to just header!

@choldgraf
Copy link
Collaborator Author

Ping on this one - any other changes people would like to see here?

@choldgraf
Copy link
Collaborator Author

ok cool let's merge this one in, then, and iterate from there!

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

3 participants