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

Fixes #2313 Update Citation Style Configuration #2316

Merged
merged 8 commits into from
Apr 28, 2023
Merged

Conversation

tadean
Copy link
Contributor

@tadean tadean commented Mar 30, 2023

Description

The seboettg/citeproc-php package has updated in its 2.6.0 release to begin using the citation-style-language/locales and citation-style-language/styles packages for its CSL rules. These packages were not previously available on packagist and it used to be a manual step to install them.

Previously, Quickstart was using academicpuma/locales (an older copy of the same locale files that was available on packagist) for its locale information, to prevent needing to install them manually.

Now that these styles are available on packagist, we may want to consider refactoring how we include the styles. This PR updates the Publication content type to use citation-style-language/locales and modifies our configuration style entities to prefer referencing CSL styles in the citation-style-language/styles package by name rather than including the xml directly.

Modifications are made to the style configuration form to allow either referencing a CSL style by name, or by custom XML. Most of our managed styles now refer to the citation-style-language/styles by reference. Bluebook Law Review currently still uses custom XML as the citation-style-language/styles version of bluebook law review does not support bibliographies The custom version of quickstart's Bluebook Law Review managed style has been modified to support bibliographies.

Spike: Feedback is desired on the best way to include references to the stylesheets, given there are a very large number of options in citation-style-language/styles; dropdown built via cached introspection of the files, specifying name manually, etc.

Related issues

#2313

How to test

  • enable az_publication
  • create some publications
  • verify citations are still rendered correctly
  • visit /admin/config/az-quickstart/settings/az-publication/az_citation_style
  • create a new style or edit one
  • For the new style, enter a stylesheet name or custom CSL (see Citation Style Language Repository for examples. Enter stylesheet names without the .csl extension, e.g. modern-language-association
  • change the default site style at /admin/config/az-quickstart/settings/az-publication to your style and verify they work correctly

Types of changes

Arizona Quickstart (install profile, custom modules, custom theme)

  • Patch release changes
    • Bug fix
    • Accessibility, performance, or security improvement
    • Critical institutional link or brand change
    • Adding experimental module
    • Update experimental module
  • Minor release changes
    • New feature
    • Breaking or visual change to existing behavior
    • Upgrade experimental module to stable
    • Enable existing module by default or database update
    • Non-critical brand change
    • New internal API or API improvement with backwards compatibility
    • Risky or disruptive cleanup to comply with coding standards
    • High-risk or disruptive change (requires upgrade path, risks regression, etc.)
  • Other or unknown
    • Other or unknown

Drupal core

  • Patch release changes
    • Security update
    • Patch level release (non-security bug-fix release)
    • Patch removal that's no longer necessary
  • Minor release changes
    • Major or minor level update
  • Other or unknown
    • Other or unknown

Drupal contrib projects

  • Patch release changes
    • Security update
    • Patch or minor level update
    • Add new module
    • Patch removal that's no longer necessary
  • Minor release changes
    • Major level update
  • Other or unknown
    • Other or unknown

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@tadean tadean added enhancement New feature or request spike Research required labels Mar 30, 2023
@tadean tadean self-assigned this Mar 30, 2023
@tadean tadean marked this pull request as ready for review April 5, 2023 15:46
@tadean tadean requested a review from a team as a code owner April 5, 2023 15:46
joeparsons
joeparsons previously approved these changes Apr 5, 2023
Copy link
Member

@joeparsons joeparsons left a comment

Choose a reason for hiding this comment

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

Code changes look good to me. I'm glad to see that classes/methods for discovering available stylesheets are provided by the CiteProc library so you didn't have to implement any of that. 👍🏻

Adding autocomplete to the stylesheet selection form element would definitely be a nice future improvement but this version seems totally usable to me as-is and I would be in favor of merging this PR.

@joeparsons
Copy link
Member

joeparsons commented Apr 5, 2023

Suggested change from 2023-04-05

  • Add a boolean (checkbox) config entity setting and form field that indicates whether the citation style config entity contains CSL or not instead of having 2 separate text fields for the style name and custom style markup.

@tadean
Copy link
Contributor Author

tadean commented Apr 7, 2023

Todo:

  • Refactor to use boolean config switch on style loading
  • Add boolean checkbox config setting for style behavior
  • AJAXify form to transform style element based on style type

bberndt-uaz
bberndt-uaz previously approved these changes Apr 14, 2023
Copy link
Contributor

@bberndt-uaz bberndt-uaz left a comment

Choose a reason for hiding this comment

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

Tested in Probo and everything looks good for this PR!

However, I did notice a potential issue with the way pages are displayed by the MLA style:

image

image

@tadean
Copy link
Contributor Author

tadean commented Apr 14, 2023

Interesting - thanks @bberndt-uaz ! I'll take a look at that and see what's going on in the style.

@tadean
Copy link
Contributor Author

tadean commented Apr 14, 2023

@bberndt-uaz it seems that MLA uses the minimal-two CSL page numbering style. It looks like the intended output of this field should be 140-50, removing the redundant one, so it seems some transformation of this number is expected, but it doesn't seem like the zero should be dropped. I'll do some testing and see if this is an issue in the CSL processor we're using.

@joeparsons
Copy link
Member

@tadean Is going to create a follow-up issue for addressing the page number issue.

@joeparsons joeparsons merged commit 8d33a3e into main Apr 28, 2023
@joeparsons joeparsons deleted the feature/2313 branch April 28, 2023 20:48
@trackleft trackleft mentioned this pull request Jun 16, 2023
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.7.x only enhancement New feature or request spike Research required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants