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

[VUFIND-747] Create Breadcrumbs view helper. #4236

Merged
merged 62 commits into from
Mar 15, 2025

Conversation

demiankatz
Copy link
Member

@demiankatz demiankatz commented Feb 7, 2025

This PR adds a view helper that uses templates to render breadcrumb trails. This makes it easier to customize and modify breadcrumbs globally, and makes breadcrumb construction more readable in individual templates.

For now, the helper manipulates $this->layout()->breadcrumbs for backward compatibility, but it could be modified to use its own internal string instead of the layout at some point in the future if we want to make it more self-contained.

TODO

  • Finish porting templates to use the helper
  • Run full test suite
  • Update v12 deprecation ticket to note RecordLinker::getBreadcrumbHtml deprecation
  • Update changelog when merging
  • Resolve VUFIND-747 when merging

@demiankatz demiankatz added the architecture pull requests that involve significant refactoring / architectural changes label Feb 7, 2025
@demiankatz demiankatz added this to the 11.0 milestone Feb 7, 2025
@demiankatz demiankatz marked this pull request as draft February 7, 2025 17:53
Copy link
Member Author

@demiankatz demiankatz left a comment

Choose a reason for hiding this comment

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

Thanks, @sturkel89 -- I have resolved the conflict here and have also reviewed your latest updated templates. They all look correct, but I had a few minor stylistic comments. You can use the buttons in my comments below to apply the changes and save yourself the trouble of making the edits yourself, and then you can pull the latest down to your environment when you're ready to work on more.

sturkel89 and others added 2 commits March 7, 2025 14:59
…fy.phtml

Co-authored-by: Demian Katz <demian.katz@villanova.edu>
…html

Co-authored-by: Demian Katz <demian.katz@villanova.edu>
@demiankatz demiankatz requested a review from EreMaijala March 13, 2025 19:28
@demiankatz
Copy link
Member Author

All tests are now passing!

Copy link
Contributor

@EreMaijala EreMaijala left a comment

Choose a reason for hiding this comment

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

With the unused translations @demiankatz mentioned removed, this looks good to me!

@demiankatz demiankatz merged commit 724bed3 into vufind-org:dev Mar 15, 2025
6 checks passed
@demiankatz demiankatz deleted the vufind-747 branch March 15, 2025 12:23
@demiankatz
Copy link
Member Author

Thanks, @EreMaijala, I cleaned up the language files and added some unit tests before merging this.

ckaz pushed a commit to finc/vufind that referenced this pull request Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
architecture pull requests that involve significant refactoring / architectural changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants