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

Fix figures so more than one link can be made to figures #822

Merged
merged 1 commit into from Jul 20, 2016

Conversation

nickevansuk
Copy link
Contributor

@nickevansuk nickevansuk commented Jun 9, 2016

Behaviour can be reproduced with multiple links created to a figure: without this change only the first one renders.


This change is Reviewable

@halindrome
Copy link
Contributor

:lgtm:

Previously, nickevansuk wrote…

Fix figures so more than one link can be made to figures

Behaviour can be reproduced with multiple links created to a figure: without this change only the first one renders.


Review status: 0 of 1 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@marcoscaceres
Copy link
Member

@nickevansuk, code looks good. Can you help me think of a test that we can add? I've not experienced this myself, so would like to make sure we have something we can see and test against (to make sure we don't accidentally regress, etc).

@marcoscaceres marcoscaceres mentioned this pull request Jul 20, 2016
@marcoscaceres
Copy link
Member

Going to merge this in to prevent it from rotting away. But we need a test for it.

@marcoscaceres marcoscaceres merged commit 5b8388b into w3c:develop Jul 20, 2016
marcoscaceres added a commit that referenced this pull request Jul 21, 2016
* develop:
  v4.1.2
  docs: deprecation warning should point to wiki (#875)
  fix: increase contrast for .parameters th and .exceptions th text (#883)
  chore(package): update promise-polyfill to version 6.0.0 (#881)
  fix(respec2.css): th code inherit color (closes #877) (#880)
  Fix(figures): figures so more than one link can be made to figures (#822)
  chore(package): update snyk to version 1.17.1 (#876)
@r12a
Copy link
Contributor

r12a commented Jul 21, 2016

We need a similar fix for links to section headings. Should i raise a different issue for that, or should we fix it here?

Also, what's needed for a test? Does someone need to create a small document and put it somewhere?

@marcoscaceres
Copy link
Member

On 21 Jul 2016, at 6:55 PM, r12a notifications@github.com wrote:

We need a similar fix for links to section headings. Should i raise a different issue for that, or should we fix it here?

Different issue please.

Also, what's needed for a test?

Please see tests/spec/ folder (find the equivalent spec for the .js file being changed). I can help write them, but just need to understand the code/problem. But take a look, it's pretty straight forward.

To run tests, you need to run:

npm run test:karma

Does someone need to create a small document and put it somewhere?

No. The doc can be generated dynamically.


You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub, or mute the thread.

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.

None yet

4 participants