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

Back To Top Link #2990

Merged
merged 53 commits into from
Feb 28, 2024
Merged

Back To Top Link #2990

merged 53 commits into from
Feb 28, 2024

Conversation

adi-unni
Copy link
Contributor

@adi-unni adi-unni commented Jan 9, 2024

What is the context of this PR?

Fixes: #1599

The back-to-top link is a requested component for the Design System. The button takes you from wherever you are on the page to the top of the page or wherever you specify.

The default options for both are "Back to Top" taking you to "#top" however, this can be modified to you to the contents with the description "Scroll to contents" for example. To modify the text of the button, edit the description parameter. Similarly, to modify the jump location, edit the anchor parameter. When making a custom point to anchor, make sure to add an id to the element you want to jump to which matches the id in the anchor parameter.

As per the requirements, the button will appear towards the bottom of the screen when the user scrolls two window heights down from the anchor div. When the user reaches the link on the page, it will take the form of the link on the page.

Note: Position of the text of the button when sticky is based on the position when it is a link on the on the page. This in turn is based on the container it is in.

How to review this PR

Check out the back to top button examples and make sure they are working correctly. Also test resizing the window to make sure the BTT links margins move accordingly

Checklist

This needs to be completed by the person raising the PR.

  • I have selected the correct Assignee
  • I have linked the correct Issue

@adi-unni adi-unni self-assigned this Jan 9, 2024
@adi-unni adi-unni linked an issue Jan 9, 2024 that may be closed by this pull request
Copy link

netlify bot commented Jan 9, 2024

Deploy Preview for ons-design-system-preview ready!

Name Link
🔨 Latest commit e7d21ab
🔍 Latest deploy log https://app.netlify.com/sites/ons-design-system-preview/deploys/65df3995e88e2a00085e0179
😎 Deploy Preview https://deploy-preview-2990--ons-design-system-preview.netlify.app/components/back-to-top/example-full-page-back-to-top
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@rmccar rmccar left a comment

Choose a reason for hiding this comment

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

Few minor things here and I think the formatting of your examples needs looking at. Theres also a few of my previous comments that haven't been addressed

@rmccar
Copy link
Contributor

rmccar commented Feb 20, 2024

Id recommend adding the eslint extension to your VScode and changing the "render whitespace" setting to "all" which will allow you to easily spot some of these things

@adi-unni
Copy link
Contributor Author

Think this focus state might be a bit big. Also I think because of this we are losing the black underline that is used on other focused elements
Screenshot 2024-02-14 at 13 44 56

Talked to Joe about the width being full / almost full width it is deemed to be as intended. To match the skip-to-content component, the text underline is being added back in on focus.

If you want to match skip-to-content wouldn't you want to have the whole thing highlighted?

Joe has confirmed this is what the intended behaviour should be. I will work on adding this in

Copy link
Contributor

@rmccar rmccar left a comment

Choose a reason for hiding this comment

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

The "backToTop" files should be renamed "back-to-top" to be consistent with other filenames of the same type

@adi-unni
Copy link
Contributor Author

Think this focus state might be a bit big. Also I think because of this we are losing the black underline that is used on other focused elements
Screenshot 2024-02-14 at 13 44 56

Talked to Joe about the width being full / almost full width it is deemed to be as intended. To match the skip-to-content component, the text underline is being added back in on focus.

If you want to match skip-to-content wouldn't you want to have the whole thing highlighted?

Joe has confirmed this is what the intended behaviour should be. I will work on adding this in

An update on this. This would not be a good solution for screens which are wide / have wide windows. Joe has requested additional UX work to be done to make a decision on this before it is implemented.

@adi-unni
Copy link
Contributor Author

Think this focus state might be a bit big. Also I think because of this we are losing the black underline that is used on other focused elements
Screenshot 2024-02-14 at 13 44 56

Talked to Joe about the width being full / almost full width it is deemed to be as intended. To match the skip-to-content component, the text underline is being added back in on focus.

If you want to match skip-to-content wouldn't you want to have the whole thing highlighted?

Joe has confirmed this is what the intended behaviour should be. I will work on adding this in

An update on this. This would not be a good solution for screens which are wide / have wide windows. Joe has requested additional UX work to be done to make a decision on this before it is implemented.

Final decision was to make the highlighted bar full width for consistency between components. Where the screen is too wide, this is being dealt with as an edge case and doesn't impact usability.

@adi-unni adi-unni merged commit 367afbc into main Feb 28, 2024
9 checks passed
@adi-unni adi-unni deleted the feature/1599/back-to-top-link branch February 28, 2024 14:46
@rmccar rmccar changed the title ⬆️ Back To Top Link ⬆️ Back To Top Link Mar 6, 2024
rmccar pushed a commit that referenced this pull request Mar 6, 2024
alessioventuriniAND added a commit that referenced this pull request Apr 4, 2024
…wcag 22 level aaa (#3119)

* ⬆️  Back To Top Link  ⬆️  (#2990)

* Page template for length

* Add macro

* Add component foundations

* Build stylesheet

* Add additional functionality

* Improved functionality

* Fix border and layout

* Fix resize behaviour

* Changes to CSS

* Add more tests

* Update VR Tests

* Change to full width

---------

Co-authored-by: Alessio Venturini <112873190+alessioventuriniAND@users.noreply.github.com>

* Description list not structured properly for screen readers (#3053)

* initial change

* update test

* update test

* update comment

* update comment

* Update placeholder text colour (#3062)

* Fix/3004/icon in footer not clickable (#3036)

* Fix checkbox appearance on safari (#3066)

* Adding the change in readme (#3067)

* updated youtube video to a more accessible one

* updated reference files

---------

Co-authored-by: Aditya Unnithan_ONS <56112669+adi-unni@users.noreply.github.com>
Co-authored-by: Precious Onyenaucheya <86783201+precious-onyenaucheya-ons@users.noreply.github.com>
Co-authored-by: rmccar <42928680+rmccar@users.noreply.github.com>
Co-authored-by: Bali Birch-Lee_ONS <149602681+balibirchlee@users.noreply.github.com>
Co-authored-by: SriHV <123635670+SriHV@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component A component in the ONS Design System
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Back to top link
4 participants