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

PR: Add anchors, direct links and buttons to dropdowns, and open and scroll to them automatically #207

Merged
merged 5 commits into from
Dec 6, 2020

Conversation

CAM-Gerlach
Copy link
Member

Pull Request Checklist

  • Read and followed this repo's Contributing Guidelines
  • Based your PR on the latest version of the correct branch (master or 3.x)
  • Checked your writing carefully for correct English spelling, grammar, etc
  • Described your changes and the motivation for them below
  • Noted what issue(s) this pull request resolves, creating one if needed

Description of Changes

Adds id anchors to each dropdown (e.g. FAQ question), allowing them to be linked to directly, along with JS that automatically scrolls to them, adjusts for the navbar and gives them some margin, and opens them. I also added link widgets to each one that behave and are styled similarly to the ones next to section headers, that allow easily copying and pasting the links, and bespoke custom classes to each FAQ question to give each a short, unique and permanently linkable name. This is specifically needed for issue #203 and PR #206 so that the FAQ questions can be linked for the side, but should be very useful in general so we can easily link users asking common questions directly to them, instead of just to the section.

Once executablebooks/sphinx-panels#56 is merged, this can be simplified to remove much of the custom setup code that relies on bespoke classes and JS hacks, and use names/ids directly instead, but I've designed the code and docs so the transition, when it happens, won't actually affect the links and will only need relatively straightforward changes.

Either this will need to be merged first and PRs #205 and #206 rebased to add the class/id names, or vice versa.

Issue(s) Resolved

Fixes #204

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Tried on Netlify and it's working great, thanks @CAM-Gerlach!

@CAM-Gerlach
Copy link
Member Author

Added a minor tweak that should improve reliability of the scrolling. Otherwise, this can be merged once @juanis2112 tests it and is happy with the style, since I can just add the right directive to the two FAQ PRs so they'll have their unique anchor ids once they get merged.

Style-wise, as a baseline I made the link anchors match the section header anchor links that serve the same function, though I'm not sure I'm sold on the look. If you all prefer, we could use our blue color instead, which would look less out of place with the FAQ its part of (at the cost of being less cohesive with the other links).

Copy link
Contributor

@juanis2112 juanis2112 left a comment

Choose a reason for hiding this comment

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

I'll suggest using the blue color for the links

@CAM-Gerlach
Copy link
Member Author

CAM-Gerlach commented Dec 6, 2020

Done, it looks way better with the blue. Unless either of you have any last-minute objections, I'll go ahead and merge once the checks pass and I verify Netlify looks good, since we agreed on it otherwise and both of you approved it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for direct links to individual FAQ questions that expand them
3 participants