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.

Sign up for GitHub

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

Jump to bottom

layout fix for #1235 #1236

Closed
wants to merge 1 commit into from
Closed

layout fix for #1235 #1236

wants to merge 1 commit into from

Conversation

dagnelies
Copy link
Contributor

@dagnelies dagnelies commented Nov 8, 2023 •
edited
Loading

For #1235

@daattali
Copy link
Owner

daattali commented Nov 8, 2023

Does this fix also take into account still supporting the original reason for ba46ef8 ? In other words, is #576 still fixed as well? Or does this PR only fix the current issue?

@dagnelies
Copy link
Contributor Author

Yup. Both fixed

@daattali
Copy link
Owner

daattali commented Nov 8, 2023

Great, I'll test it later, thanks! Looking at the code changes, I'd like to change the <main> to a <div>, because there are already divs further down that have role="main", and I don't think it's good practice to have <main> and inside of it another div with role="main"

@dagnelies
Copy link
Contributor Author

Sure. Do as you like. Please pay attention that the CSS rule body > main { flex: 1 } should only target the main area of the body, not the nav header or footer.

@dagnelies
Copy link
Contributor Author

dagnelies commented Nov 9, 2023 •
edited
Loading

you could even drop this outer <main> container directly and apply the flex:1 to the main content container instead (<div class="container ..." role="main">) ...or rename that one as "main" tag instead of "role".

<main>
    <div class="intro-header"></div>
    <div class=" container-md " role="main">
   ...
</main>

To:

<div class="intro-header"></div>
<main class=" container-md ">

No CSS change.

@daattali
Copy link
Owner

daattali commented Nov 10, 2023 •
edited
Loading

@dagnelies Thanks for all your work on this. I just updated the layouts so that every page now has a <main> tag - however, it's not in the exact same hierarchical location within the DOM on each page type. Would applying flex:1 to the main tag in the new code solve the problem?

@dagnelies
Copy link
Contributor Author

The CSS rule body > main { flex: 1 } makes the main grow to fill the empty page if necessary. As long as there is a <main> directly under the <body>, anywhere, what's after the main will appear at the bottom.

@daattali daattali closed this Nov 10, 2023
@daattali daattali mentioned this pull request Nov 10, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dagnelies @daattali