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

Add new anchors in my-settings and handle offset sub-menu height #3032

Merged
merged 5 commits into from
Aug 10, 2020

Conversation

kimsible
Copy link
Contributor

@kimsible kimsible commented Aug 3, 2020

All links in the user dropdown-menu refered to /my-account/settings with an anchor #video-settings, does not scroll exactly to the the target element, plus when we are on my-settings all these links can't work properly since they have the same anchor.

It also handle the offset with the fixed sub-menu.

image

@rigelk rigelk added the UI non-trivial UI changes, that might need discussion label Aug 3, 2020
@kimsible kimsible force-pushed the fix/anchor-shorcuts-settings branch from 45d5600 to 12d8d58 Compare August 3, 2020 20:55
@@ -114,6 +114,8 @@ export class AppComponent implements OnInit, AfterViewInit {
let resetScroll = true
const eventsObs = this.router.events

this.viewportScroller.setOffset([0, 81]) // sub-menu-height
Copy link
Owner

Choose a reason for hiding this comment

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

Should only be applied if targeting my-account no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my-account and about ? Maybe we would add any anchors in about/instance

Copy link
Owner

Choose a reason for hiding this comment

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

yes

@kimsible
Copy link
Contributor Author

One thing left, for example when we request two times on /my-account/settings#notifications and we scrolled between the two requests we would like to go back to the anchor #notifications scroll position with the scrollToAnchor. My knowledge of Angular Router is still limited, would you have an idea @rigelk @Chocobozzz how to proceed ? which routee event we should subscribe ?

@Chocobozzz Chocobozzz merged commit 4a53fc8 into Chocobozzz:develop Aug 10, 2020
@Chocobozzz
Copy link
Owner

One thing left, for example when we request two times on /my-account/settings#notifications and we scrolled between the two requests we would like to go back to the anchor #notifications scroll position with the scrollToAnchor

Sorry I don't know

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI non-trivial UI changes, that might need discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants