-
Notifications
You must be signed in to change notification settings - Fork 43
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
Add 'Back to top' links to main and about pages #135
Add 'Back to top' links to main and about pages #135
Conversation
Previously, the app mounted to the `#root` element. The meant that we couldn't also _really_ use it for back-to-top navigation as the fragment in the url name would look weird. Instead, this renames the containing element to `#top` for navigation purposes and we instruct React to mount via a class based querySelector on the new `.app-root` class. Issue: UKHSA-Internal#97
This is largly un-styled, but the proof-of-concept for the logic is there. Issue: UKHSA-Internal#97
This makes the links only appear on `/` and `/about`. Issue: UKHSA-Internal#97
This includes making a small, internal-only, component to hold the svg content for the up arrow and improving the logic on how the overlay and inline modes made themselves visible. The back-to-top link SVG, HTML & CSS comes from https://github.com/alphagov/govuk-design-system/blob/master/views/partials/_back-to-top.njk and https://github.com/alphagov/govuk-design-system/blob/master/src/stylesheets/components/_back-to-top.scss but doesn't appear to be in the `govuk-react-jsx` package, so I included it directly for simplicity. Issue: UKHSA-Internal#97
<svg role="presentation" focusable="false" class="back-to-top" xmlns="http://www.w3.org/2000/svg" width="13" height="17" viewBox="0 0 13 17"> | ||
<path fill="currentColor" d="M6.5 0L0 6.5 1.4 8l4-4v12.7h2V4l4.3 4L13 6.4z"></path> | ||
</svg>Back to top |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice touch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs a couple of docs / comment to clarify the options and the usage to other contributors.
Great job, @MikeCoats. Thank you for the contribution.
Yeah, no worries. I’ll fire in some more details when I’m back at my computer. |
How are we doing with the docs / comments @MikeCoats? I'm hoping to include this PR in the next release tomorrow. |
I’ve done about half of the changes and I’ll finish the rest and push them tonight if that’s alright?
…Sent from my iPhone
On 3 May 2020, at 11:49, Pouria Hadjibagheri ***@***.***> wrote:
How are we doing with the docs / comments @MikeCoats?
I'm hoping to include this PR in the next release tomorrow.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
Add javadoc style comments for components and functions and some inline comments to explain the why, not just the how. Issue: UKHSA-Internal#97
@xenatisch That's the comments added. If they need a bit more clarification, let me know - I'll be WFH from about 8:15-8:30 tomorrow, and I'll update them as quick as I can. |
@MikeCoats It's all good. Thank you! :) |
Essentials updates and improvements - fixes #79, #80, #125, #128, #134, #135, and #139. With thanks to @MikeCoats, @richardTowers, and @dracos for their contributions.
This PR adds a 'back to top' link to the main and about pages. It will appear once you've scrolled down by half a page and will stay overlayed on the screen until you hit the bottom of the page, where it then nestles between the content and the footer. It appears to work well at mobile resolutions and aspect ratios too.
Screenshots
No link at top of page
Overlayed link when scrolled down page
Inline link when scrolled down page
No link at top of page on mobile
Overlayed link when scrolled down page on mobile
Inline link when scrolled down page on mobile
Fixes #97