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 multiple QR code for available address and show/hide functionality #3928

Merged
merged 3 commits into from
Mar 13, 2025

Conversation

nitinawari
Copy link
Contributor

@nitinawari nitinawari commented Mar 12, 2025

User description

close #3927

  • if multiple addresses there set by user preference will given to BCH > BTC > ETH by default
blt-crypto-QR.mp4

image


PR Type

Enhancement, Tests


Description

  • Added functionality to toggle and switch between multiple QR codes for crypto addresses.

  • Implemented buttons for BTC, BCH, and ETH with dynamic QR code generation.

  • Enhanced UI with a blurred QR code toggle and placeholder for missing addresses.

  • Updated styles and added copy-to-clipboard functionality for crypto addresses.


Changes walkthrough 📝

Relevant files
Enhancement
crypto_donation.html
Added toggle and switcher for crypto QR codes                       

website/templates/includes/crypto_donation.html

  • Added buttons for BTC, BCH, and ETH with dynamic QR code generation.
  • Implemented a toggle to show/hide QR codes with blurred and clear
    states.
  • Enhanced address display logic with fallback to a default BLT donation
    address.
  • Refactored JavaScript for better modularity and initialization logic.
  • +221/-52
    tiny-cards.html
    Improved styling for crypto address cards                               

    website/templates/includes/tiny-cards.html

  • Adjusted border thickness for better visibility.
  • Increased font size for crypto address display.
  • +2/-2     
    profile.html
    Integrated crypto donation section into profile                   

    website/templates/profile.html

  • Added a section header for "Crypto Addresses".
  • Included the updated crypto donation template.
  • +4/-1     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis 🔶

    3927 - Partially compliant

    Compliant requirements:

    • Add multiple QR codes for available addresses.
    • Implement a switcher for toggling between QR codes.
    • Include an eye icon to hide QR codes by default.
    • Add dummy placeholders for missing addresses.

    Non-compliant requirements:

    Requires further human verification:

    • Verify the UI/UX functionality of the toggle and switcher in a browser.
    • Confirm the QR code generation and visibility toggle behavior.
    • Ensure the placeholders and copy-to-clipboard functionality work as expected.
    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Possible Issue

    The updateQRCode function does not validate the address parameter for potential malicious input, which could lead to unintended behavior or vulnerabilities.

    function updateQRCode(address) {
        if (address && address !== "None") {
            const qrUrl = `https://api.qrserver.com/v1/create-qr-code/?data=${address}&size=400x300`;
            cryptoQrImage.src = qrUrl;
            cryptoQrImageClear.src = qrUrl;
            return true;
        }
        return false;
    UI/UX Consistency

    The QR code toggle functionality might not be intuitive for users. Ensure the toggle behavior is clear and user-friendly.

    qrContainer.addEventListener('click', () => {
        if (qrBlurred.classList.contains('hidden')) {
            qrBlurred.classList.remove('hidden');
            qrIcon.classList.remove('hidden');
            qrClear.classList.add('hidden');
        } else {
            qrBlurred.classList.add('hidden');
            qrIcon.classList.add('hidden');
            qrClear.classList.remove('hidden');

    Copy link
    Contributor

    PR Code Suggestions ✨

    @DonnieBLT
    Copy link
    Collaborator

    That looks great! If you are going to have the QR code like that then there is no need for the icon. It can be visible. Thank you!

    @nitinawari
    Copy link
    Contributor Author

    nitinawari commented Mar 12, 2025

    That looks great! If you are going to have the QR code like that then there is no need for the icon. It can be visible. Thank you!

    what was you suggesting maybe I misunderstood the problem and also instead of creating switcher I used buttons

    @DonnieBLT DonnieBLT enabled auto-merge (squash) March 13, 2025 05:44
    @DonnieBLT DonnieBLT merged commit 95214b2 into OWASP-BLT:main Mar 13, 2025
    10 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    Add toggle & switcher to show/hide multiple QR codes for crypto addresses
    2 participants