Place a scrollable button to the top of the page #2243

kazuhitonakayama · 2020-12-20T08:28:28Z

Why:

When a site is viewed from a smartphone or other device, the absence of the relevant button may cause difficulty for users.
please refer to the issue #1894

What's being changed:

Before

After

When you scroll 100px, its scroll button will be displayed.

Check off the following:

All of the tests are passing.
I have reviewed my changes in staging. (look for the deploy-to-heroku link in your pull request, then click View deployment)
For content changes, I have reviewed the localization checklist
For content changes, I have reviewed the Content style guide for GitHub Docs.

welcome · 2020-12-20T08:28:30Z

Thanks for opening this pull request! A GitHub docs team member should be by to give feedback soon. In the meantime, please check out the contributing guidelines.

janiceilene · 2020-12-21T16:21:15Z

@kazuhitonakayama Thanks for opening a PR! I see that this is currently a draft, please feel free to let me know when you're ready for this to be put up for review 💖

kazuhitonakayama · 2020-12-21T23:31:17Z

@janiceilene
Thank you for your comment...！✨
Ok! I see that.
I will code and complete the pull request as soon as possible. And I will tell you about it.!

When a site is viewed from a smartphone or other device, the absence of the relevant button may cause difficulty for users.

kazuhitonakayama · 2020-12-25T06:37:24Z

@janiceilene
My apologies for the delay in getting the PR done!
It's finally done, so please Review🙇‍♂️🙇‍♂️🙇‍♂️

JasonEtco

This is awesome @kazuhitonakayama 👏 A great idea, and well executed! I left a couple of notes below, please let me know if I can clarify anything.

One other thing - there's a lot of custom CSS here, and that's fine but we can move a lot of it to use Primer's utility classes (like position-fixed, etc). I can be more specific if that's helpful!

JasonEtco · 2020-12-28T16:17:23Z

includes/scroll-button.html

+<div href="" class="arrow-for-scrolling-top" id="js-scroll-top">
+    <span class="arrow"></span>
+</div>
+{% include scripts %}


Including scripts here will duplicate them on some pages - do you mind removing this line? For example, in the default layout its already included via includes/small-footer.html!

Suggested change

{% include scripts %}

@JasonEtco
Thank you for your advice！
I remove the including scripts {% include scripts %}

JasonEtco · 2020-12-28T16:20:21Z

includes/scroll-button.html

+<div href="" class="arrow-for-scrolling-top" id="js-scroll-top">
+    <span class="arrow"></span>
+</div>


Can we make this button a semantic <button>? That way it follows things like tab ordering, hover/focus/click behavior - all really helpful for screen readers and keyboard controls!

Suggested change

<div href="" class="arrow-for-scrolling-top" id="js-scroll-top">

<span class="arrow"></span>

</div>

<button class="arrow-for-scrolling-top" id="js-scroll-top">

{% octicon "chevron-up" %}

</button>

This might require some additional style tweaking, but it's worth it!

I also used an Octicon here, so that we use a consistent visual language.

@JasonEtco
Thank you for your advices🙇‍♂️
I rewrite 'div' to 'button' and changed to use octicon!

JasonEtco · 2020-12-28T16:21:14Z

javascripts/scroll-up.js

+  window.addEventListener('scroll', function () {
+    const y = document.documentElement.scrollTop // get the height from page top
+    if (y < 100) {
+      PageTopBtn.classList.remove('show')
+    } else if (y >= 100) {
+      PageTopBtn.classList.add('show')
+    }
+  })


I love this! Great addition 👏

@JasonEtco I'm really glad to hear that✨

JasonEtco · 2020-12-28T16:22:09Z

stylesheets/scroll-button.scss

+    -webkit-transition:1s;
+    -moz-transition:1s;
+    -ms-transition:1s;
+    -o-transition:1s;


We can omit the compatibility prefixes, our Webpack pipeline generates those for us!

Thank you!
I omitted the compatibility prefixes🙇‍♂️
@JasonEtco

simpsoka · 2020-12-28T17:15:31Z

Thanks for adding this, @kazuhitonakayama! Curious if there's a way to have this button only shows up if the user has scrolled past a certain point? The example you show has it showing up when the user is already at the top of the page. Ideally, it shows up when you've scrolled past the first fold.

github-actions · 2021-01-04T17:41:14Z

This PR is stale because it has been open 7 days with no activity and will be automatically closed in 3 days. To keep this PR open, update the PR by adding a comment or pushing a commit.

kazuhitonakayama · 2021-01-05T07:08:43Z

Thank you for your comments!!

github-actions

👋 Hey there spelunker. It looks like you've modified some files that we can't accept as contributions. The complete list of files we can't accept are:
translations/**
lib/rest/static/**
.github/workflows/**
.github/CODEOWNERS
translations/**
assets/fonts/**
data/graphql/**
lib/graphql/**
lib/redirects/**
lib/rest/**
lib/webhooks/**

You'll need to revert all of the files you changed in that list using GitHub Desktop or git checkout origin/main <file name>. Once you get those files reverted, we can continue with the review process.

Alyana38 · 2021-02-22T12:18:04Z

Благодарю за поддержку!были трудности с переводом страница,и случайно нажаты не те клавиши!ребята вы большие молодцы!!А я только учусь,не ругайте пожалуйста 😊

heiskr

Let's ship it

heiskr · 2021-02-22T22:39:58Z

stylesheets/scroll-button.scss

+button.arrow-for-scrolling-top {
+    opacity: 0;
+    transition: 1s;
+    background-color: #0367d6;


We might want to use a primer variable

javascripts/scroll-up.js

kazuhitonakayama · 2021-03-27T10:26:53Z

@heiskr
I apologize for not being able to complete the project. Also, thank you very much for fixing and merging the code.🙇‍♂️

janiceilene · 2021-03-29T20:46:30Z

@kazuhitonakayama I love the button 👏 Thank you so much for all the work you were able to put in 💝

kazuhitonakayama · 2021-03-30T15:59:15Z

@kazuhitonakayama I love the button 👏 Thank you so much for all the work you were able to put in 💝

@janiceilene So glad to hear that, and to be able to contribute to the Github we all love!

Abhishek577270 · 2021-03-30T16:46:03Z

Hello world

Place a scrollable button to the top of the page

389fde5

When a site is viewed from a smartphone or other device, the absence of the relevant button may cause difficulty for users.

kazuhitonakayama force-pushed the add-button-for-scrolling-to-top branch from 2fd2ee9 to 389fde5 Compare December 25, 2020 06:29

kazuhitonakayama marked this pull request as ready for review December 25, 2020 06:34

kazuhitonakayama requested a review from a team as a code owner December 25, 2020 06:34

janiceilene added the engineering Will involve Docs Engineering label Dec 28, 2020

JasonEtco reviewed Dec 28, 2020

View reviewed changes

github-actions bot added the stale There is no recent activity on this issue or pull request label Jan 4, 2021

JasonEtco removed the stale There is no recent activity on this issue or pull request label Jan 4, 2021

chiedo assigned JasonEtco Jan 5, 2021

kazuhitonakayama mentioned this pull request Jan 7, 2021

Add a go to top button #1894

Closed

chiedo added the never-stale Do not close as stale label Jan 8, 2021

chiedo removed the never-stale Do not close as stale label Jan 26, 2021

janiceilene added the waiting for review Issue/PR is waiting for a writer's review label Feb 5, 2021

chiedo removed the waiting for review Issue/PR is waiting for a writer's review label Feb 9, 2021

kazuhitonakayama force-pushed the add-button-for-scrolling-to-top branch 2 times, most recently from f3aebd7 to 389fde5 Compare February 11, 2021 09:03

kazuhitonakayama requested a review from a team as a code owner February 11, 2021 09:16

github-actions bot requested changes Feb 11, 2021

View reviewed changes

kazuhitonakayama force-pushed the add-button-for-scrolling-to-top branch from 8ca202f to 389fde5 Compare February 11, 2021 09:19

kazuhitonakayama added 3 commits February 11, 2021 18:56

fix conflict

9216387

change div element to button element

521ce8b

omit the border of scroll-top-button

b66cf97

janiceilene added the waiting for review Issue/PR is waiting for a writer's review label Feb 18, 2021

heiskr previously approved these changes Feb 22, 2021

View reviewed changes

Merge branch 'main' into add-button-for-scrolling-to-top

8b6e1be

heiskr enabled auto-merge February 22, 2021 22:40

Merge branch 'main' into add-button-for-scrolling-to-top

05e6bf0

heiskr reviewed Feb 22, 2021

View reviewed changes

javascripts/scroll-up.js Show resolved Hide resolved

heiskr disabled auto-merge February 22, 2021 23:02

chiedo assigned heiskr and unassigned JasonEtco Feb 23, 2021

Update scroll-up.js

bae890e

heiskr dismissed stale reviews from ghost and themself via bae890e February 23, 2021 16:51

heiskr previously approved these changes Feb 23, 2021

View reviewed changes

Update scroll-up.js

3ea7737

heiskr dismissed their stale review via 3ea7737 February 23, 2021 16:55

heiskr approved these changes Feb 23, 2021

View reviewed changes

heiskr enabled auto-merge February 23, 2021 16:59

Merge branch 'main' into add-button-for-scrolling-to-top

e0bf94a

heiskr added ready to merge This pull request is ready to merge and removed waiting for review Issue/PR is waiting for a writer's review labels Feb 23, 2021

heiskr disabled auto-merge February 23, 2021 18:18

heiskr merged commit bf31f3e into github:main Feb 23, 2021

kazuhitonakayama deleted the add-button-for-scrolling-to-top branch April 29, 2023 07:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Place a scrollable button to the top of the page #2243

Place a scrollable button to the top of the page #2243

kazuhitonakayama commented Dec 20, 2020 •

edited

Loading

welcome bot commented Dec 20, 2020

janiceilene commented Dec 21, 2020

kazuhitonakayama commented Dec 21, 2020

kazuhitonakayama commented Dec 25, 2020

JasonEtco left a comment

JasonEtco Dec 28, 2020

kazuhitonakayama Feb 11, 2021

JasonEtco Dec 28, 2020 •

edited

Loading

kazuhitonakayama Feb 11, 2021

JasonEtco Dec 28, 2020

kazuhitonakayama Feb 11, 2021

JasonEtco Dec 28, 2020

kazuhitonakayama Feb 11, 2021

simpsoka commented Dec 28, 2020

github-actions bot commented Jan 4, 2021

kazuhitonakayama commented Jan 5, 2021

github-actions bot left a comment

Alyana38 commented Feb 22, 2021

heiskr left a comment

heiskr Feb 22, 2021

kazuhitonakayama commented Mar 27, 2021

janiceilene commented Mar 29, 2021

kazuhitonakayama commented Mar 30, 2021

Abhishek577270 commented Mar 30, 2021

-<div href="" class="arrow-for-scrolling-top" id="js-scroll-top">
-    <span class="arrow"></span>
-</div>
+<button class="arrow-for-scrolling-top" id="js-scroll-top">
+  {% octicon "chevron-up" %}
+</button>

Place a scrollable button to the top of the page #2243

Place a scrollable button to the top of the page #2243

Conversation

kazuhitonakayama commented Dec 20, 2020 • edited Loading

Why:

What's being changed:

Before

After

Check off the following:

welcome bot commented Dec 20, 2020

janiceilene commented Dec 21, 2020

kazuhitonakayama commented Dec 21, 2020

kazuhitonakayama commented Dec 25, 2020

JasonEtco left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JasonEtco Dec 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simpsoka commented Dec 28, 2020

github-actions bot commented Jan 4, 2021

kazuhitonakayama commented Jan 5, 2021

github-actions bot left a comment

Choose a reason for hiding this comment

Alyana38 commented Feb 22, 2021

heiskr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kazuhitonakayama commented Mar 27, 2021

janiceilene commented Mar 29, 2021

kazuhitonakayama commented Mar 30, 2021

Abhishek577270 commented Mar 30, 2021

kazuhitonakayama commented Dec 20, 2020 •

edited

Loading

JasonEtco Dec 28, 2020 •

edited

Loading