Conversation
|
Can one of the admins verify this patch? |
1 similar comment
|
Can one of the admins verify this patch? |
| import java.util.regex.Matcher; | ||
| import java.util.regex.Pattern; | ||
|
|
||
| public class CitationFormatter { |
There was a problem hiding this comment.
I'm confused about what this class is for. We don't have in text citations, just a list of references, so we don't need any in-text citation formatting.
| */ | ||
| @Override | ||
| @Transient | ||
| public String htmlRenderedText() { |
There was a problem hiding this comment.
the citations are in a biblio block, not a text block. So I don't think this is right.
|
I think you misunderstood the ticket. This is for the biblio blocks, not the text blocks. |
jdamerow
left a comment
jdamerow
left a comment
There was a problem hiding this comment.
this should use the class in the 150 branch.
jdamerow
left a comment
jdamerow
left a comment
There was a problem hiding this comment.
The references classes are not used it seems. The ReferenceDisplayFormatter should be used to build a reference in a certain format (not hand-build in the AP Citation formatter). The idea is that different formatters call the reference display formatter in a different order, with different parameters (e.g. parentheses or no parentheses for the year).
The IReferenceMetadataRegistry would be called to get a provider for a specific metadatatype (e.g. APA). The provider would then handle building the metadata string using the display formatter. Either in the template or the controller, the provider registry would be called to get a provider. The bibliobock itself should not worry about the display part (otherwise, it would get bigger and bigger with each new citation style we add).
jdamerow
left a comment
jdamerow
left a comment
There was a problem hiding this comment.
It might make sense to do the rendering in Javascript rather than in the backend. Let's maybe give this one a try: https://github.com/larsgw/citation.js. It should make the backend less complex and might give us more options in the frontend.
| * The registry is set via Spring autowiring when this component is initialized. | ||
| */ | ||
| @Component | ||
| public class ReferenceMetadataRegistryHolder { |
| */ | ||
| @Override | ||
| @Transient | ||
| public String renderAPAReferences() { |
There was a problem hiding this comment.
this should not be here. the block should not do the rendering, that's what the APAReferenceMetadataProvider is for.
3 participants
Guidelines for Pull Requests
If you haven't yet read our code review guidelines, please do so, You can find them here.
Please confirm the following by adding an x for each item (turn
[ ]into[x]).Please provide a brief description of your ticket
Display of references on slides
references should be in apa format
VSPC-276
Anything else the reviewer needs to know?
... describe here ...