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

remove boxed links in PDF #4254

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

thechampagne
Copy link

  • This remove boxed links in the specification PDF file; it make it easy to read.
    Screenshot from 2025-02-04 03-09-45

  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

Copy link

google-cla bot commented Feb 4, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@munificent
Copy link
Member

What do you think, @eernstg?

Copy link
Member

@eernstg eernstg left a comment

Choose a reason for hiding this comment

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

Looks good to me! I'd certainly prefer to get rid of all those boxes when the PDF file is viewed.

It could be argued that this takes away the hint that there is a link in the first place, and this would make the PDF less navigable. However, I don't think that's a big problem: The links are generated at very specific locations (table of contents, section refs, index), and it is probably not hard to get a reasonably good intuition about which spots are clickable.

@eernstg
Copy link
Member

eernstg commented Feb 7, 2025

@thechampagne, can you settle the CLA issue?

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.

3 participants