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

style(convertPayload.ts): adding formatting text on publish field whe… #1564

Merged
merged 1 commit into from Jan 17, 2024

Conversation

Danfx
Copy link
Contributor

@Danfx Danfx commented Jan 17, 2024

Added to format text when CBOR is selected on publish type field. The text is formatted like JSON type, follow:
Screenshot from 2024-01-17 11-09-38

@ysfscream ysfscream added enhancement New feature or request desktop MQTTX Desktop labels Jan 17, 2024
@ysfscream ysfscream added this to the v1.9.9 milestone Jan 17, 2024
@@ -55,7 +55,7 @@ const convertPayload = async (payload: string, currentType: PayloadType, fromTyp
if (currentType === 'Base64') {
$payload = convertBase64($payload, 'encode')
}
if (currentType === 'JSON') {
if (currentType === 'JSON' || currentType === 'CBOR') {
Copy link
Member

Choose a reason for hiding this comment

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

One personal preference for a more recommended writing style:

Suggested change
if (currentType === 'JSON' || currentType === 'CBOR') {
if (['JSON', 'CBOR'].includes(currentType)) {

@ysfscream
Copy link
Member

ysfscream commented Jan 17, 2024

@danf137 Thank you again for your code contribution. Additionally, I have implemented the MQTTX CLI CBOR conversion at #1565. Feel free to test and use it.

@ysfscream ysfscream merged commit 1518139 into emqx:main Jan 17, 2024
2 checks passed
@Danfx Danfx deleted the cbor_payload_encoder branch January 17, 2024 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
desktop MQTTX Desktop enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants