feat: add explicit buttons to heading titles #377
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Resyntax analyzed 1 file in this pull request and found no issues.
| if (info.style.display == "none") | ||
| /* Clicking the information button shows the explanation element: */ | ||
| const heading = elem.querySelector('.heading-source'); | ||
| if (heading) { |
There was a problem hiding this comment.
Why is this test needed? Can it ever fail?
There was a problem hiding this comment.
Technically, yes. If a Scribble writer inserts, say, a h3 element with custom attributes that happen to match the structure that we are looking for, but did not have the corresponding .heading-source button, then heading would be null.
There was a problem hiding this comment.
Thanks for working on this, I agree it's good to make this more discoverable than it currently is. 😄
I wonder if it's a bit too visually noisy / distracting to show these buttons all the time though...? Some sites that do this sort of thing only show the buttons for a single header when you hover on the header text. Sure, it reduces discoverability a bit, but you're still bound to encounter them after a bit of reading time. Just a suggestion from my initial reaction to your screenshots.
About vertical alignment, the buttons seem to be a few pixel lower than I'd expect. Here's GitHub vs. this PR (with an added line at the text baseline):
GitHub:
This PR:
I would recommend nudging the icons up a few pixels so that none of them drop below the text baseline. If you're not sure how, I could also give it a try if you like.
sorawee
force-pushed
the
heading-buttons
branch
from
c8a4906 to
8f0e2ad
Compare
|
Still not perfect, but this should be a bit better. 
(It can be made better, but that will require a percentage value that is "ugly" instead. This is my best attempt without resorting to using the ugly numbers.) |
There was a problem hiding this comment.
Resyntax analyzed 1 file in this pull request and found no issues.
|
I'll second @jryans's suggestion and rationale to show the buttons only when mousing over the title. |
This commit adds two buttons to each heading title. These buttons are by default hidden, but they will appear on hovering on the heading titles. The first button is a "link here" button, which adjusts the URL of the current page to include an anchor to link to that heading title, similar to how GitHub makes each heading title in README documents a link to add an anchor. The second button is to view Scribble source and internal link. Previously, clicking heading titles serves this functionality, but without an indicator that heading titles can be clicked, the feature is not discoverable. So this commit makes a transition toward an explicit button.
sorawee
force-pushed
the
heading-buttons
branch
from
8f0e2ad to
6d74d1c
Compare
|
I made the update as requested. Since we now show the buttons only on hovering over the heading titles, I removed the adjustments to make the buttons smaller*, to help with discoverability. [*] well, there's still a place that needs this adjustment, since it would be too large with the full size. |
There was a problem hiding this comment.
Resyntax analyzed 1 file in this pull request and found no issues.
|
Thanks for the update! This looks good to me. |
This PR fixes a couple of issues related to the button group. - On `#lang scribble/base`, only show the "Link to here" button, and do so with correct styling. - Make it possible for the button group to exceed the right margin to preserve the previous HTML layout. - Place anchor at the beginning of the headings, not at the end, so that on navigation, we will see the full heading. Thanks to @mflatt who reported some of the issues above and helped with testing. Fixes #381 make the button group unbreakaable do not include the "i" button when it does not make sense Add anchors at the beginning so that we see the entire heading This partially reverts ##377 make the button group be able to exceed the right margin switch from the link emoji to "#" . .
This commit adds two buttons to each heading title.
These buttons are by default hidden, but they will
appear on hovering on the heading titles.
The first button is a "link here" button, which adjusts the URL of the
current page to include an anchor to link to that heading title, similar
to how GitHub makes each heading title in README documents a link
to add an anchor.
The second button is to view Scribble source and internal link.
Previously, clicking heading titles serves this functionality,
but without an indicator that heading titles can be clicked,
the feature is not discoverable. So this commit makes a
transition toward an explicit button.
Example screenshots:
Page title
Section title