Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Aug 20, 2024

Proposed Changes

https://community.rockrms.com/ideas/2009/allow-customizable-colors-for-qr-codes

This change allows for RGBA hex values in the QR Code generator. These values can come in as RRGGBB, or RRGGBBAA hex values.

Types of changes

What types of changes does your code introduce to Rock?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality, which has been approved by the core team)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • [x ] This is a single-commit PR. (If not, please squash your commit and re-submit it.)
  • [ x] I verified my PR does not include whitespace/formatting changes -- because if it does it will be closed without merging.
  • [ x] I have read the Contributing to Rock doc
  • [x ] By contributing code, I agree to license my contribution under the Rock Community License Agreement
  • [ x] Unit tests pass locally with my changes
  • [ x] I have added any required unit tests or integration tests that prove my fix is effective or that my feature works
  • [ x] I have included updated language for the Rock Documentation (if appropriate)

Further comments

I made these changes with 6 and 8 digit RGBA values in mind, not 3 or 4-length values (like CSS allows). If that is something people would also like to see in the change, I can accommodate that. I think it would be useful to incorporate the RockColor utilities class into this because it has a Dictionary of CSS hex values and their keywords.

The only downside of that is that you still need to convert the hex values into a byte[] because of QRCoder's method overloads.

Documentation

New documentation should be applied once these changes are made so that users understand the flexibility they now have when creating QR codes.

Copy link
Member

@nairdo nairdo left a comment

Choose a reason for hiding this comment

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

Thanks Noah. Can you check out these small adjustments? Let me know if you have any questions/concerns.

}
}
catch ( Exception )
catch (Exception)
Copy link
Member

Choose a reason for hiding this comment

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

Please retain the whitespace here and in the if below.

}
}

private byte[] HexToByteArray( string hex )
Copy link
Member

Choose a reason for hiding this comment

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

Even though these two new methods are private, it's a good idea to add method /// documentation to describe what the intention of each method is -- but especially this one.

Copy link
Member

Choose a reason for hiding this comment

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

FYI: We'll be minimally adding a small section next to this section in the Developer 101 guide around here:
https://community.rockrms.com/developer/book/16/16/content#referencingfilesandimages

Copy link
Member

@nairdo nairdo Aug 22, 2024

Choose a reason for hiding this comment

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

@nlBayside Also check the foreground and background colors when used with SVG vs PNG... I think you might have one of them reversed.

                var lightColor = HexToByteArray( foregroundColor );
                var darkColor = HexToByteArray( backgroundColor );

The lightColor should be the background color while the darkColor is the foreground (for the PNG output type).

image

@ghost
Copy link
Author

ghost commented Aug 26, 2024

Let me know if those changes suffice! I had it done in the browser editor. So if I missed something I can go back and fix that

@nairdo nairdo merged commit 6cffbe2 into SparkDevNetwork:develop Aug 28, 2024
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.

2 participants