-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Hash change docs #1168
Hash change docs #1168
Conversation
This seems to work with linking to specific sections of a 😃 👍 |
docs/index.html
Outdated
if(currentHash) { | ||
var hashElement = document.getElementById(currentHash); | ||
if (hashElement) { | ||
hashElement.scrollIntoView(); |
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.
Change hashElement.scrollIntoView();
to hashElement.scrollIntoView({ behavior: 'smooth' });
for a buttery transition ⛷️
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.
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.
It worked correctly when you clicked "supported specifications" because that was the only time it hit the scrollIntoView()
function as far as I can tell.
docs/CONTRIBUTING.md
Outdated
@@ -25,15 +25,15 @@ The following table lists the ticket type labels we use when there is work to be | |||
|Ticket type label |Description | | |||
|:----------------------------------|:-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | |||
|L0 - security |A security vulnerability within the Marked library is discovered. | | |||
|L1 - broken |Valid usage results in incorrect output compared to [supported specifications](README.md#specifications) OR causes marked to crash AND there is no known workaround for the issue. | | |||
|L1 - broken |Valid usage results in incorrect output compared to [supported specifications](#README.md#specifications) OR causes marked to crash AND there is no known workaround for the issue. | |
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.
nit: I think the urls to md files should being with a slash #/README.md#specifications
.
This is a common pattern for js routers so that you know its linking to a different route and not a normal anchor on the page. Thoughts?
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.
We determine whether it is a route or anchor by the existence of /.md$/
but we could change it to /^\\/
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.
I also realized, we can use #/README
and drop the .md
altogether which will read a little bit better.
@joshbruce Any objections to this? |
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.
Would really like to consider an alternative to the SPA and using the hash for anything other than inline anchors.
There are still some issues after this fix:
|
Hash change docs
Marked version: 0.3.19
Markdown flavor: Markdown.pl|CommonMark|GitHub Flavored Markdown|n/a
Description
Contributor
Committer
In most cases, this should be a different person than the contributor.