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 anchors to arguments #2499

Merged
merged 6 commits into from
Apr 29, 2024
Merged

Add anchors to arguments #2499

merged 6 commits into from
Apr 29, 2024

Conversation

hadley
Copy link
Member

@hadley hadley commented Apr 25, 2024

Fixes #2228

@glin I can't request a review, but I'd appreciate your thoughts on this PR, particularly the visual styling.

@hadley
Copy link
Member Author

hadley commented Apr 25, 2024

Sample styling:

Screenshot 2024-04-25 at 11 30 30

@glin
Copy link

glin commented Apr 25, 2024

I didn't really review the code changes, but functionally this looks good to me. I've definitely wished it was possible to link to arguments a few times in the past.

For styling, I did think the border stood out a bit at first glance. Maybe a thinner border would look good, or maybe more spacing between the border and text?

Or alternatively, what I see often on doc sites is that the active anchor is kept visible and sometimes visually emphasized. This would probably be a more subtle option. Here's an example from some docs I was just looking at: https://docs.posit.co/rspm/news/package-manager/#posit-package-manager-2023120

No strong opinions on this though. But also as a side note, I like the visual indicator for active anchors, and think it would be useful to add to heading anchors at some point too.

@hadley
Copy link
Member Author

hadley commented Apr 28, 2024

I like the idea of making the anchor links stay visible in general; but I think arguments will need a bit more than. But I take your point that this visual treatment might be a bit heavy, and I'll brainstorm some alternatives.

@hadley
Copy link
Member Author

hadley commented Apr 28, 2024

I tried a few other visual treatments (including a light background and changing the text colour), but it's hard to be prominent enough without causing potential accessibility issues. So I refined the initial approach using a narrower border, more whitespace, and switching colours (to the primary colour, which is also used for links, giving a nicer visual connection IMO). I also liked the idea of showing the anchor, which lead to a few visual tweaks there too.

Screenshot 2024-04-29 at 10 48 52

@hadley hadley merged commit 5f9606b into main Apr 29, 2024
12 checks passed
@hadley hadley deleted the argument-anchors branch April 29, 2024 19:47
@glin
Copy link

glin commented Apr 30, 2024

The changes look great! The added whitespace and narrower border help a lot for me. Also, definitely appreciate the anchor change.

SebKrantz pushed a commit to SebKrantz/pkgdown that referenced this pull request Jun 1, 2024
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.

Add link anchors to arguments
2 participants