Skip to content

Support UTF-8 encoding for non-ASCII QR data (e.g. Korean, emoji)#284

Merged
rosskhanas merged 2 commits intorosskhanas:masterfrom
GraceKim527:master
Sep 4, 2025
Merged

Support UTF-8 encoding for non-ASCII QR data (e.g. Korean, emoji)#284
rosskhanas merged 2 commits intorosskhanas:masterfrom
GraceKim527:master

Conversation

@GraceKim527
Copy link
Copy Markdown
Contributor

Related Issue

#283

✨ Improvement Suggestion

  • Convert the string to UTF-8 bytes using TextEncoder or a similar UTF-8 encoding utility.
  • Explicitly specify Byte mode when generating the QR code: addData(utf8Bytes, 'Byte')

🔁 Refactor

  • Moved UTF-8 encoding logic to utils/encodeStringToUtf8Bytes.js
  • Moved byte-to-string conversion logic to utils/bytesToBinaryString.js

@GraceKim527 GraceKim527 changed the title Feat: support UTF-8 encoding for non-ASCII QR data (e.g. Korean, emoji) Support UTF-8 encoding for non-ASCII QR data (e.g. Korean, emoji) Sep 2, 2025
Copy link
Copy Markdown
Owner

@rosskhanas rosskhanas left a comment

Choose a reason for hiding this comment

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

Thank you for your PR 🙂

Comment thread src/utils/encodeStringToUtf8Bytes.js Outdated
@@ -0,0 +1,33 @@
export function encodeStringToUtf8Bytes(input) {
if (typeof input !== "string") return [];
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I think this check makes no sense, as it takes value from react component and it is a required prop.

Comment thread src/utils/encodeStringToUtf8Bytes.js Outdated
Comment thread src/index.js Outdated
Comment thread src/index.js
/>
);
});
export const QRCode = forwardRef(
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

There is biome config with 140 chars in 1 line, I think you have some smaller value in your config, ideally not to change all this haha

@rosskhanas rosskhanas merged commit 853a491 into rosskhanas:master Sep 4, 2025
@rosskhanas
Copy link
Copy Markdown
Owner

감사합니다, @GraceKim527 🙂

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.

2 participants