-
Notifications
You must be signed in to change notification settings - Fork 360
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 "copy to clipboard" button and use it for citations #4289
Refactor "copy to clipboard" button and use it for citations #4289
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @ThoWagen -- haven't done any hands-on testing yet, but see below for a few minor comments from a read-through of the code.
themes/bootstrap5/templates/Helpers/copy-to-clipboard-button.phtml
Outdated
Show resolved
Hide resolved
themes/bootstrap5/templates/RecordDriver/DefaultRecord/toolbar.phtml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks good and works correctly for me; see below for a couple of whitespace issues which I plan to take care of myself prior to merging.
module/VuFind/src/VuFind/View/Helper/Bootstrap5/CopyToClipboardButton.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me; I'll merge this as soon as tests pass!
The main purpose of this PR was to add "copy to clipboard" buttons to the citations.
At the same time the "copy to clipboard" buttons got refactored (removal of jQuery, JavaScript moved into a .js file, success/error message as popover and new design for permalinks).