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

show more link jumps problem #8501

Merged
merged 3 commits into from Apr 7, 2021
Merged

show more link jumps problem #8501

merged 3 commits into from Apr 7, 2021

Conversation

lyndaidaii
Copy link
Contributor

@lyndaidaii lyndaidaii commented Apr 6, 2021

Summary of the changes (in less than 80 characters):

before changes:
Clicking the Show More link clears the URL fragment and goes to the top of the page.

after changes:
The README should expand but the viewport should stay where it is.

Addresses #8461

@@ -517,7 +517,7 @@
@Html.Raw(Model.ReadMeHtml)
</div>

<a href="#" role="button" data-toggle="collapse" class="icon-link" data-target="#readme-more"
<a href="#show-readme-container" role="button" data-toggle="collapse" class="icon-link" data-target="#readme-more"
Copy link
Member

Choose a reason for hiding this comment

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

Hmm.. if this is the fix it seems that the click event is bubbling out and the default (navigation) behavior is remaining.
https://stackoverflow.com/a/866587

This is just making the viewport jump to this link, right?

Copy link
Member

Choose a reason for hiding this comment

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

I am confused by the GIF -- does the value in the address bar change?

Copy link
Contributor Author

@lyndaidaii lyndaidaii Apr 6, 2021

Choose a reason for hiding this comment

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

when we click show-more, viewport jump to this link(https://localhost/packages/embeddedReadmeTestApp/#show-readme-container insteadof https://localhost/packages/embeddedReadmeTestApp/#).

Copy link
Member

Choose a reason for hiding this comment

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

This should not be necessary. This is a button (as evidenced by the role attribute not a traditional link. A navigation event should not even occur. See that Stack Overflow above. Is the event bubbling out of this click handler?

$("#show-readme-more").click(function (e) {
showLess.collapse("toggle");
e.preventDefault();
});

I'm wondering if perhaps this behavior regressed when we updated jQuery...

Maybe we just need return false in that method?

Ideally nothing happens in the address bar when you click this button. It's not really a link, per se.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like return false works, nothing happens in the address bar when we click this button.

@@ -74,6 +74,7 @@ $(function () {
showLess.on('show.bs.collapse', function (e) {
e.stopPropagation();
});
return false;
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 not where I would expect it. I would expect the return false in the event (click) handler. Maybe this very long SO answer will help clarify the situation for you?
https://stackoverflow.com/a/30473685
Let's sync offline if things are still confusing :)

@lyndaidaii lyndaidaii merged commit c7a2355 into dev Apr 7, 2021
@ryuyu ryuyu mentioned this pull request Apr 8, 2021
9 tasks
lyndaidaii added a commit that referenced this pull request Apr 8, 2021
lyndaidaii added a commit that referenced this pull request Apr 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants