feat: add citation popup for blog posts (fixes #3322)#3332
feat: add citation popup for blog posts (fixes #3322)#3332SyedIshmumAhnaf wants to merge 9 commits into
Conversation
There was a problem hiding this comment.
Thanks for contributing to the matrix.org website. I have automatically marked your pull request as a draft. Please restore the PR template to your PR description and follow our contributing guidelines, then undraft to let the team know the PR is ready for review.
There was a problem hiding this comment.
Thanks for contributing to the matrix.org website. I have automatically marked your pull request as a draft. Please restore the PR template to your PR description and follow our contributing guidelines, then undraft to let the team know the PR is ready for review.
|
Hello. Thank you for the detailed feedback, this is really helpful. I understand the architectural concerns raised here, especially around keeping styling within Based on your suggestions, I am planning to refactor this PR as follows:
I’ll push an updated version shortly following this approach. Also, I agree that using a macro (or shortcode) makes this more reusable across the site. I’ll incorporate that into the refactor. Let me know if there’s anything else I should consider while making these changes. |
|
Quoted from a previous comment:
Just to be clear, my suggestion was to make the component itself, not just the citation‑generation part, a macro and include it in
In addition to supporting APA, it would be nice to add support for MLA, Chicago, Harvard and possibly BibTeX within this PR. If you have any questions, feel free to reach out either here on GitHub or in #matrix.org-website:matrix.org. |
|
Thank you for the clarification, this helps a lot. I understand the intention more clearly now. I’ll make the entire citation component reusable via a macro or shortcode and include it in templates/post.html. I’ll refactor so that the macro outputs the full component. Also, regarding the different citation formats, I’ll update the implementation to also include MLA, Chicago, Harvard and BibTeX in this PR. Lastly, I’ll ensure that my component follows the existing pattern the project uses in header.html and _header.scss for toggling behavior so that it does not rely on Javascript for core functionality. I’ll push an updated version. Thanks again for the guidance. |
SyedIshmumAhnaf
left a comment
There was a problem hiding this comment.
Hello again, as per our last discussion, I have refactored the codes and pushed an update. The primary changes made are as follows:
- Using a cleaner macro structure with consistent fallback handling
- Improved accessibility in the modal
- Refactored SCSS for better readability and maintainablity
- Added a tab switching logic for different citation formats, APA, BibTex, Harvard, etc.
9311595 to
166f022
Compare
SyedIshmumAhnaf
left a comment
There was a problem hiding this comment.
As per our last discussion, I've made a few changes to the codebase to remove js based techniques and replace with a css only approach. I have also added option for other citation formats.
There was a problem hiding this comment.
I haven't addressed how each citation style conforms to its respective style guide in this review; I'll cover that in a separate review later.
Alongside the requested changes, I'd like the button's appearance updated to match the other buttons used on the website.
|
I edited your original comment to include the “closes” keyword so that merging this PR will automatically close the issue. |
|
Thanks for the detailed review. I’ve addressed the requested changes. To summarize, here is what has changed:
Would you mind taking another look when you have a moment? |
|
Thank you for d35e0b6. While I look into the formatting of each citation type, could you add support for copy-to-clipboard functionality as a progressive enhancement? |
|
Sure, I'll look into it. |
|
Hello @awtj8o81ryywg793, As previously discussed, I’ve added copy-to-clipboard support as a progressive enhancement. The citation text is still generated at build time, and the modal/tabs continue to work without JavaScript. The copy controls are hidden by default and are only enabled when the Clipboard API is available, so the no-JS fallback remains manual selection/copying from the visible citation text. I hope this gets merged soon. |
SyedIshmumAhnaf
left a comment
There was a problem hiding this comment.
Hi @awtj8o81ryywg793, thanks for the detailed feedback. I’ve pushed an update addressing all the points raised.
Changes made:
- Moved
.cite-panelinto the existing{% for fmt in formats %}loop to remove duplicated markup - Reworked the copy-to-clipboard UI to use a single shared copy button, positioned below the citation textbox
- Moved the “Copied” status next to the button for clearer feedback
- Updated the copy button styling to use the existing
.call-to-actionpattern for consistency with the rest of the site - Updated the JS to copy based on the currently selected format (via the active radio input)
The progressive enhancement approach is preserved:
- No-JS users can still access the modal and switch formats as before
- Copy controls are only enabled when the Clipboard API is available
I’ve tested this locally across all formats and everything appears to be working as expected, but happy to adjust further if needed.
Here is a screenshot of the UI for reference:
SyedIshmumAhnaf
left a comment
There was a problem hiding this comment.
Hi, thanks for the follow-up review and the UI example.
I’ve pushed another update addressing the latest feedback:
- Updated the copy interaction so the button label remains “Copy”, with status shown only via the adjacent status text
- Switched the copy button to
call-to-action(removedsecondary) and adjusted sizing to better match the modal - Applied the suggested compact pill styling to
.cite-btnand.cite-format-tabfor consistency with the provided example - Ensured all citation controls follow a consistent visual style within the modal
Everything has been tested locally (all formats + no-JS fallback) and zola build passes with no new issues.
Let me know if anything still feels off or needs further refinement.
|
Quoted from a previous comment:
Thanks! I'll review your changes and get back to you as soon as I can. |
There was a problem hiding this comment.
After stepping back from the pull request for a bit, I think that, instead of showing a single format selectable via buttons, we should mirror the design in the original issue (i.e. show all formats at once, organised in a vertical list).
I also think it might be worth moving away from using <label> elements for toggle functionality, since the popover attribute has good browser support. Also, while linking multiple <label> elements to a single <input> works, it isn't valid. See: https://developer.mozilla.org/docs/Web/HTML/Reference/Global_attributes/popover.
I apologise for the churn in this pull request. After these changes, and once we’ve revisited the formatting of the citations themselves, this should be good to go.
|
|
||
| <div class="cite-formats"> | ||
|
|
||
| {% set formats = ["apa", "mla", "chicago", "harvard", "bibtex"] %} |
There was a problem hiding this comment.
Let's move this to below the definition of cb_id on line 6.
| </div> | ||
|
|
||
| {% endmacro render %} | ||
|
|
| {{ authors }}. ({{ year }}). {{ page.title }}. Matrix.org. {{ page.permalink }} | ||
| {%- endmacro cite_apa %} | ||
|
|
||
|
|
| {{ authors }}. "{{ page.title }}." Matrix.org, {{ date_fmt }}, {{ page.permalink }}. | ||
| {%- endmacro cite_mla %} | ||
|
|
||
|
|
| {{ authors }}. "{{ page.title }}." Matrix.org. {{ date_fmt }}. {{ page.permalink }}. | ||
| {%- endmacro cite_chicago %} | ||
|
|
||
|
|
| {{ authors }} ({{ year }}) '{{ page.title }}', Matrix.org, viewed {{ date_fmt }}, <{{ page.permalink }}>. | ||
| {%- endmacro cite_harvard %} | ||
|
|
||
|
|
…ation macro structure in Zola templates keeping a standardized fallback logic for authors and dates. Refactored SCSS for better maintainability. Also, added and fixed tab switching logic for different formats. Lastly, no major changes were made regarding output formatting and instead matched zola formatting. Signed-off-by: Syed Ishmum Ahnaf syedishmum15@gmail.com
Co-authored-by: Jack S. <181536874+awtj8o81ryywg793@users.noreply.github.com> Signed-off-by: Syed Ishmum Ahnaf <syedishmum15@gmail.com>
…tion and updated section with page.taxonomies.author only. Signed-off-by: Syed Ishmum Ahnaf syedishmum15@gmail.com
…yed Ishmum Ahnaf syedishmum15@gmail.com
…s to render .cite-panel via loops. Now, using one single copy button, which matches styling with .call-to-action, below citation text. Preserved no-JS fallback. Signed-off-by: Syed Ishmum Ahnaf syedishmum15@gmail.com
…copy status in the adjacent status element only and preserved Copy button label after clipboard action. Also, applied compact pill styling and using call-to-action styling for the copy button. Signed-off-by: Syed Ishmum Ahnaf <syedishmum15@gmail.com>
Signed-off-by: Syed Ishmum Ahnaf <syedishmum15@gmail.com>
|
Hi @awtj8o81ryywg793, no worries about the churn. I’ve pushed an update implementing the latest requested refactor:
I tested this locally and checked that everything works as expected. I’ve left the citation-formatting macro outputs intact for now, since you mentioned the formatting itself will be revisited separately after this UI/structure change. |

Description
Adds a citation popup modal for blog posts and adds option to copy citation.
Related issues
Closes #3322.
Role
Individual
Timeline
No specified deadline
Signoff
Signed-off-by: Ishmum
Screenshots