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.

Sign up for GitHub

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

Jump to bottom

Only change current if a matching link is found #472

Merged
merged 1 commit into from
Oct 13, 2017
Merged

Only change current if a matching link is found #472

merged 1 commit into from
Oct 13, 2017

Conversation

petermd
Copy link
Contributor

@petermd petermd commented Oct 4, 2017

Fix for #397 to only reset current style and re-apply if a new candidate link has been found.

Envoy docs have links with unresolved anchors, this fix avoids collapsing the L3/L4 headers if not matching link for the anchor is found.

@Blendify Blendify requested a review from agjohnson October 4, 2017 14:22
@Blendify
Copy link
Member

Blendify commented Oct 4, 2017

Lgtm but I want to leave it up to @agjohnson because I do not do much Javascript.

@mattklein123
Copy link

@Blendify @agjohnson I'm fine going ahead and forking, however I'm a little confused about the "grunt" aspect of this. When I tried this patch without whatever extra stuff @petermd did it did not work for me. Can you advise so I can do the right thing in the fork? Thank you.

@Blendify Blendify self-requested a review October 13, 2017 01:52
@Blendify
Copy link
Member

The correct thing to do is run grunt build the reason we say not to do that with the Pull Request is this auto-generates files which can be an issue with multiple pull requests. Although because it works for you and you have been using it for a while I will just merge it. However, I will ask for a favor, can you submit a new pr based off after the merge that just runs grunt build. I would do it but I only have a chromebook atm and wont be able to do it.

@Blendify Blendify merged commit ee4d440 into readthedocs:master Oct 13, 2017
@mattklein123
Copy link

Thanks. @petermd any chance you can do the extra PR?

@petermd petermd deleted the avoid-auto-reset branch October 13, 2017 10:43
@petermd petermd mentioned this pull request Oct 13, 2017
Merged
Blendify pushed a commit that referenced this pull request Oct 15, 2017
@petermd @Blendify
Follow-up grunt for #472 (#476)
9d704f2 
* grunt run

* regenerate theme css
travismiller pushed a commit to broco1974/couscous-template-readthedocs that referenced this pull request Dec 4, 2017
Merge commit '6c2302472fb2c61eec9b4be3a2515117fffbfac3' into rtfd
e7bbdd2 
* commit '6c2302472fb2c61eec9b4be3a2515117fffbfac3': (253 commits)
  Add pygments_style to theme.conf
  Document new theme option
  Add option to layout.html
  Update theme.conf
  Apply clearfix on breadcrumbs
  Follow-up grunt for readthedocs#472 (readthedocs#476)
  Only change current if a matching link is found (readthedocs#472)
  Add a couple of badge icons to readme (readthedocs#471)
  Add Sphinx Framework for PyPi (readthedocs#470)
  Fix error in setup entry points (readthedocs#466)
  Added `.admonition` to the CSS (readthedocs#462)
  Use explicit UTF8 encoding (readthedocs#452)
  Add missing pygments css link
  Convert underscores to dashes.
  Reflect language in lang attribute.
  Replace master with v0.2.5 in README.rst
  Add missing dependency in dev setup: sphinxcontrib-httpdomain (readthedocs#431)
  Finalized readthedocs#418 (readthedocs#433)
  Added a link to the Sphinx documentation about setuptools entry points
  Added setuptools entry point for Sphinx
  ...
@stsewd stsewd mentioned this pull request Oct 8, 2020
Closed
This was referenced Aug 19, 2022
Open
Open
This was referenced Nov 4, 2022
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Blendify Blendify Blendify approved these changes

@agjohnson agjohnson Awaiting requested review from agjohnson

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@petermd @Blendify @mattklein123 @agjohnson