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

scroll to top functionality #5620

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,4 @@
[submodule "content-modules/opentelemetry-java-examples"]
path = content-modules/opentelemetry-java-examples
url = https://github.com/open-telemetry/opentelemetry-java-examples.git
javaexamples-pin = f9553ef
javaexamples-pin = 0f736ec
63 changes: 63 additions & 0 deletions assets/js/scrollToTop.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
document.addEventListener("DOMContentLoaded", function () {
const scrollToTopBtn = document.getElementById("scrollToTopBtn");
const footer = document.querySelector(".td-footer");
const scrollLabel = scrollToTopBtn?.querySelector(".scroll-label");
let hideTimeout;

// Check if the current page is the homepage
if (window.location.pathname === "/") {
if (scrollToTopBtn) scrollToTopBtn.style.display = "none";
return; // Exit early to prevent further execution on the homepage
}

if (!scrollToTopBtn || !footer) return;

// Function to adjust button position to avoid footer overlap
function adjustButtonPosition() {
const footerTop = footer.getBoundingClientRect().top;
const windowHeight = window.innerHeight;

if (footerTop < windowHeight + 40) {
scrollToTopBtn.style.bottom = `${windowHeight - footerTop + 40}px`;
} else {
scrollToTopBtn.style.bottom = "40px";
}
}

// Show or hide the button with a fade effect and handle label transition
function toggleButtonVisibility() {
const scrollPosition =
document.body.scrollTop || document.documentElement.scrollTop;

if (scrollPosition > 200) {
scrollToTopBtn.classList.add("visible");

// Show the label after 3-4 page scrolls
const pageHeight = window.innerHeight;
if (scrollPosition > pageHeight * 3) {
scrollToTopBtn.classList.add("show-label");
} else {
scrollToTopBtn.classList.remove("show-label");
}

// Clear any existing timeout for hiding the button
if (hideTimeout) clearTimeout(hideTimeout);

// Hide the button after 2-3 seconds of inactivity
hideTimeout = setTimeout(() => {
scrollToTopBtn.classList.remove("visible");
}, 2500);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
}, 2500);
}, 5000);

For me 2.5 seconds is to early, I suggest 5 or even more seconds for the timeout

} else {
scrollToTopBtn.classList.remove("visible");
scrollToTopBtn.classList.remove("show-label");
}

adjustButtonPosition();
}

scrollToTopBtn.onclick = function () {
window.scrollTo({ top: 0, behavior: "smooth" });
};

window.addEventListener("scroll", toggleButtonVisibility);
});
119 changes: 108 additions & 11 deletions assets/scss/_styles_project.scss
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,15 @@
margin-top: 1rem;
margin-bottom: 1rem;

> ul {
>ul {
list-style: none;
margin: 0;
padding: 0;

> li {
>li {
display: inline;
> a {

>a {
@extend .btn;
margin: 0.25rem;

Expand All @@ -47,7 +48,10 @@
.l-get-started-buttons {
@extend .l-buttons;

> ul > li > a /*, > p > a*/ {
>ul>li>a

/*, > p > a*/
{
@extend .btn-lg;
@extend .btn-secondary;
}
Expand All @@ -56,7 +60,7 @@
.l-primary-buttons {
@extend .l-buttons;

> ul > li > a {
>ul>li>a {
@extend .btn-lg;
@extend .btn-primary;
}
Expand All @@ -67,7 +71,10 @@
@extend .l-buttons;
justify-content: left;

> ul > li > a /*, > p > a*/ {
>ul>li>a

/*, > p > a*/
{
@extend .btn-lg;
@extend .btn-primary;
padding: 20px;
Expand All @@ -80,7 +87,10 @@
@extend .l-buttons;
justify-content: left;

> ul > li > a /*, > p > a*/ {
>ul>li>a

/*, > p > a*/
{
@extend .btn-lg;
@extend .btn-secondary;
padding: 20px;
Expand All @@ -95,9 +105,11 @@
@include media-breakpoint-up(md) {
padding: 0;
}

svg {
height: 48px;
}

.navbar-brand__name {
display: none;
}
Expand Down Expand Up @@ -127,6 +139,7 @@

a {
color: white;

&.external-link:after {
display: none;
}
Expand Down Expand Up @@ -167,7 +180,7 @@
.community-contribution {
text-align: center;

& > p {
&>p {
font-size: $h3-font-size;
font-weight: $headings-font-weight;
line-height: $headings-line-height;
Expand All @@ -192,6 +205,7 @@

summary {
display: block;

&::-webkit-details-marker {
display: none;
}
Expand Down Expand Up @@ -226,9 +240,11 @@
.td-byline {
margin: 0 !important;
}

.article-meta {
margin: 0;
}

p:first-of-type {
@extend .small;
opacity: 0.65;
Expand Down Expand Up @@ -321,9 +337,90 @@ details {
// Workaround for iOS and macOS Safari 17+. For details see:
// https://github.com/open-telemetry/opentelemetry.io/issues/3538

.td-content .highlight > pre {
> .click-to-copy,
> code {
.td-content .highlight>pre {

>.click-to-copy,
>code {
overflow-y: auto;
}
}

// Scroll-to-top button styling
.scroll-container {
position: fixed;
bottom: 40px;
right: 20px;
z-index: 1000;

.scroll-btn {
position: fixed;
bottom: 0;
right: 20px;
display: flex;
flex-direction: column;
align-items: center;
justify-content: center;
background-color: transparent;
border: none;
border-radius: 0;
cursor: pointer;
opacity: 0;
visibility: hidden;
transition: all 0.3s ease-in-out;
padding: 8px 16px;

&:hover {
background-color: #f1f1f1;
border: 2px solid #007bff;
border-top-right-radius: 20px;
border-bottom-left-radius: 0;
border-top-left-radius: 0;
border-bottom-right-radius: 0;
}
Comment on lines +372 to +379
Copy link
Member

Choose a reason for hiding this comment

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

you might want to add a mode for dark mode as well


.scroll-icon {
font-size: 18px;
color: #007bff;
transition: transform 0.3s ease-in-out, color 0.3s ease-in-out;

&:hover {
color: #0056b3;
transform: scale(1.2);
}
}

.scroll-label {
font-size: 12px;
font-family: "Roboto", Arial, sans-serif;
font-weight: 500;
color: #333;
opacity: 0;
transform: translateY(10px);
transition: opacity 0.5s ease-in-out, transform 0.5s ease-in-out;
text-align: center;
}

&.show-label .scroll-label {
opacity: 1;
transform: translateY(0);
}

&.visible {
opacity: 1;
visibility: visible;
}
}
}

@media (max-width: 768px) {
.scroll-container {
.scroll-btn {
bottom: 20px;
right: 10px;

.scroll-icon {
font-size: 16px;
}
}
}
}
2 changes: 1 addition & 1 deletion content-modules/opentelemetry-java-examples
2 changes: 2 additions & 0 deletions layouts/partials/hooks/body-end.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@
{{ partial "script.html" (dict "src" "js/tracing.js") -}}
{{ end -}}
{{ partial "script.html" (dict "src" "js/navScroll.js") -}}
{{ partial "scroll-to-top.html" }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{{ partial "scroll-to-top.html" }}
{{ partial "scroll-to-top.html" -}}

{{ partial "script.html" (dict "src" "js/scrollToTop.js") -}}
6 changes: 6 additions & 0 deletions layouts/partials/scroll-to-top.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<div class="scroll-container">
<button id="scrollToTopBtn" class="scroll-btn btn" aria-label="Scroll to top" title="Scroll to top">
<i class="fas fa-chevron-up scroll-icon"></i>
<span class="scroll-label">Back to Top</span>
Copy link
Member

Choose a reason for hiding this comment

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

this is never visible for me

Copy link
Author

Choose a reason for hiding this comment

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

It comes up after 3 to 4 scrolls which is mostly for long content pages did you try such page, maybe the registry page? @svrnm

Copy link
Member

Choose a reason for hiding this comment

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

what's the purpose of showing the arrow earlier than the text?

Copy link
Contributor

Choose a reason for hiding this comment

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

For the sake of i18n, I'd drop the English label. I think that using an icon is clear enough. WDYT @svrnm?

Copy link
Author

@bintus-ux bintus-ux Dec 5, 2024

Choose a reason for hiding this comment

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

As pointed out by @theletterf i ought to only display the text after 3-4 page scrolls so I decided to only display the arrow until the page scroll triggers smoothly transitioning into view the label text? Or how do you suggest I go about it @svrnm

Copy link
Member

Choose a reason for hiding this comment

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

having that label is an accessibility feature for me, not everyone might immediately understand the meaning of this button/we can not assume that since we understand it everyone does. And again, that's also why I dislike the 3-4 page scrolls until it shows up.

Although I have to admit I didn't think about the localized pages.

Since we have conflicting opinions between maintainers here, I suggest we do not over-engineer the problem for now and block @bintus-ux any longer, so let's do the following:

  • Remove the "Back to top" label for now
  • Raise a new issue that we want to have it added to the scroll to top functionality.

For me this would be good enough, wdyt? @chalin @theletterf

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, good plan! 🙌🏻

Copy link
Contributor

Choose a reason for hiding this comment

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

Btw, the need-of-rebase issue and one suggestion I made in #5620 (review) are still pending.

Copy link
Author

Choose a reason for hiding this comment

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

Yea that sounds good, fixes will be done today, thanks again!

</button>
</div>
Loading