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

Switch to Grid and Footer #97

Merged
merged 14 commits into from
Nov 4, 2024
Merged

Switch to Grid and Footer #97

merged 14 commits into from
Nov 4, 2024

Conversation

wilwade
Copy link
Contributor

@wilwade wilwade commented Nov 1, 2024

Problem

Needed to do the footer, but had to change a good amount to make it work.

Closes #83

Solution

  • Added a grid
  • Moved the old menu icons to the left as a column
  • Shifted around a few things for the grid.
  • Kept the resize
  • Kept the sidebar animation
  • Kept the touch controls for mobile (swipe to get the sidebar)
  • Added the footer

Screenshots (optional):

Here are a few, but you really have pull and run locally

Screen Shot 2024-11-04 at 11 59 29-fullpage
Screen Shot 2024-11-04 at 11 59 47-fullpage
Screen Shot 2024-11-04 at 12 00 07-fullpage
Screen Shot 2024-11-04 at 12 00 17-fullpage
image

image image

}

@media (min-width: 800px) {
.frqcy-header {
padding: 0 80px;
padding: 0 62px;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This helps align the Frequency logo by default with the sidebar

@@ -604,66 +604,3 @@ function playground_text(playground, hidden = true) {
document.scrollingElement.scrollTo({ top: 0, behavior: "smooth" });
});
})();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't want to do any of this anymore. Perhaps we'll bring back the scrolling header disappearing later.

@@ -2,6 +2,53 @@

@import "variables.css";

:target {
scroll-margin-top: calc(var(--header-height) + 20px);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make it so that the anchor links aren't hidden by the header.

scroll-margin-top: calc(var(--header-height) + 20px);
}

/* GRID */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to move to grid to make everything work. No, I didn't want to, but it was the cleanest way.

@@ -34,44 +81,22 @@ a > .hljs {
will want to reposition the viewport in a weird way.
*/
overflow-x: clip;
transition: 200ms;
}

/* Menu Bar */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Menu bar changes removed a ton of css.

}

[dir="rtl"] .sidebar {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not currently looking to move the sidebar side for rtl. Would require grid setup

@@ -682,11 +650,11 @@ ul#searchresults span.teaser em {
.theme-popup {
position: absolute;
left: 10px;
top: var(--menu-bar-height);
top: 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This way the theme popup is with the icon


<div id="page-wrapper" class="page-wrapper">

<div class="page">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this extra div, but otherwise most of these are moving things around.


<div class="page">
<div id="menu-bar-hover-placeholder"></div>
<div id="menu-bar" class="menu-bar sticky">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved outside to the grid structure


<div style="clear: both"></div>
</nav>
<div id="page-wrapper" class="page-wrapper">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's the content wrapper now

@wilwade wilwade marked this pull request as ready for review November 4, 2024 17:10
@wilwade wilwade requested a review from a team November 4, 2024 17:10
@wilwade wilwade changed the title WIP: Dp/footer 83 Switch to Grid and Footer Nov 4, 2024
@wilwade wilwade mentioned this pull request Nov 1, 2024
16 tasks
Copy link

@saraswatpuneet saraswatpuneet left a comment

Choose a reason for hiding this comment

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

Looks good to me, ran locally

The inner container seems to be sizing in and out based on content. readability is not consistent, suggestion would be to pin the forward and back button pin to bottom of page, or pin footer

Copy link
Contributor

@shannonwells shannonwells left a comment

Choose a reason for hiding this comment

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

All works for me, but I have a couple of notes, which I don't consider blocking until we get some more finalized design:

  • Was there supposed to be a dark version of the footer?

  • I don't know if this is a regression, but the prev/next chapter buttons are off-center for this view:

Screenshot 2024-11-04 at 1 32 23 PM

@wilwade wilwade merged commit 04ca880 into developer-portal Nov 4, 2024
1 check passed
@wilwade wilwade deleted the dp/footer-83 branch November 4, 2024 21:41
@wilwade wilwade mentioned this pull request Nov 4, 2024
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.

3 participants