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

Refactor subject pages to have re-usable macros #9786

Merged

Conversation

jimchamp
Copy link
Collaborator

@jimchamp jimchamp commented Aug 22, 2024

Closes #9785

Extracts three subject page components into macros:

  • ProlificAuthors.html: The list of prolific authors on a subject
  • PublishingHistory.html: The publishing history graph for a subject
  • RelatedSubjects.html: Links to related subject pages

Additionally, removes unreferenced renderPublishers jsdef function.

The existing carousel is a custom_carousel template, and hasn't modified.

Technical

renderSubjects in the RelatedSubjects macro is no longer a jsdef function. This function worked properly when called from the browser console, but wasn't referenced in our codebase.

The renderAuthors jsdef function was not included in the ProlificAuthors macro. This function did not work properly when called from the console, due to an Uncaught ReferenceError: page is not defined error. Also, it isn't referenced in our codebase.

Testing

  1. Before deploying these changes, navigate to a subject page.
  2. Open another tab.
  3. Deploy the branch.
  4. Open the same subject page in your new tab. Content, styling, and functionality should be the same when the tabs are compared.

Screenshot

Stakeholders

@github-actions github-actions bot added the Priority: 2 Important, as time permits. [managed] label Aug 22, 2024
@mekarpeles mekarpeles merged commit c23d4a8 into internetarchive:master Aug 26, 2024
4 checks passed
@jimchamp jimchamp deleted the 9785/feature/subject-page-macros branch August 27, 2024 18:53
@cdrini cdrini changed the title Create macros for subject pages Refactor subject pages to have re-usable macros Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: 2 Important, as time permits. [managed]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Encapsulate subject page components as macros
2 participants