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

Added a QR icon in Share Modal that opens to QR code on click #9233

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

merwhite11
Copy link
Contributor

@merwhite11 merwhite11 commented May 7, 2024

Closes #452

Feature: Adds a QR code icon to the share modal on books page. On icon click, a new tab opens with the QR code image.
Refactor: Changed styling of Share Modal so that for small -> tablet sized screens, the share modal icons appear larger and in two rows.

Technical

Installed qrcode dependency in requirements.txt
Imported qrcode and io in api.py for use in create_qrcode handler.

Testing

docker compose run --rm home make test : passing with warnings
docker compose run --rm home pytest openlibrary/plugins/importapi/tests/test_import_validator.py: passing with warnings
docker compose run --rm home npm run lint: passing with deprecation warnings

Screenshot

I can change the size of the icons if these are too large. I can also change the breakpoint for when to switch from large 2-row icons, to smaller single row icons. Currently I have the icons in 2 rows for everything tablet and smaller.
Screenshot 2024-05-07 at 2 01 36 PM

Stakeholders

@jimchamp
Thank you for the help!

@merwhite11 merwhite11 force-pushed the 452/feature/add-qr-to-share-modal branch from 3416158 to 2e9a182 Compare May 8, 2024 19:24
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 0% with 30 lines in your changes are missing coverage. Please review.

Project coverage is 16.11%. Comparing base (db9ead2) to head (936d6e1).
Report is 141 commits behind head on master.

Files Patch % Lines
openlibrary/plugins/openlibrary/js/modals/index.js 0.00% 27 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9233      +/-   ##
==========================================
+ Coverage   15.89%   16.11%   +0.22%     
==========================================
  Files          90       91       +1     
  Lines        4732     4785      +53     
  Branches      824      829       +5     
==========================================
+ Hits          752      771      +19     
- Misses       3468     3498      +30     
- Partials      512      516       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@merwhite11 merwhite11 force-pushed the 452/feature/add-qr-to-share-modal branch 2 times, most recently from 43e71a7 to 52650ba Compare May 8, 2024 19:35
@merwhite11 merwhite11 force-pushed the 452/feature/add-qr-to-share-modal branch from f317528 to 06d8feb Compare May 22, 2024 21:06
@merwhite11 merwhite11 force-pushed the 452/feature/add-qr-to-share-modal branch from fc03faf to 6939cc6 Compare May 22, 2024 21:07
@merwhite11
Copy link
Contributor Author

@jimchamp Could you review this when you get the chance? Thanks !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add QR code to Share Modal
3 participants