search

meshtastic / web

Meshtastic Web Client
https://client.meshtastic.org
GNU General Public License v3.0
229 stars 86 forks source link

Channels URL for QR Codes Aren't Properly Formatted #259

Closed AddisonTustin closed 1 month ago

AddisonTustin commented 1 month ago

What did you do?

The syntax of a URI/URL requires that the fragment (the portion of the URI that follows a #) should be at the very end of the URI. When beginning to work on meshtastic/Meshtastic-Android#953 I noticed that since ?add=true followed the fragment, Java's native URI feature does not properly parse add=true as a query param, and instead includes it as part of the fragment.

There's some cleanup I'd like to do as a part of meshtastic/Meshtastic-Android#953 to use the URI directly rather than doing manual string processing. So this becomes a small blocker.

Expected Behavior

The query params (?add=true) should be included in the URI before the fragment.

Link to URI Generic Syntax (Library of Congress).

Current Behavior

When constructing the channelsUrl ?add=true is appended to the end of the URI.

Participation

Hunter275 commented 1 month ago

@AddisonTustin you marked that you were willing to do a PR for this, is that in the works?

AddisonTustin commented 1 month ago

~Haven't started the PR yet.~ I filed a similar issue in Meshtastic-Apple and want to make sure that there's agreement across the different usages before putting out any changes.

edit: I can at least throw up a draft pr (#260) pending the outcome of that conversation.

Hunter275 commented 1 month ago

@AddisonTustin where did this land? I believe the iOS app has been updated?

AddisonTustin commented 1 month ago

I think I can probably submit the pr draft I had. Though it might need to be updated slightly to handle extracting the channel config from old/legacy URLs?

If that's not a concern then the pr should be good for review.

Hunter275 commented 1 month ago

I think I can probably submit the pr draft I had. Though it might need to be updated slightly to handle extracting the channel config from old/legacy URLs?

If that's not a concern then the pr should be good for review.

I don't think we need to worry about legacy URLs

AddisonTustin commented 1 month ago

Cool, marked #260 as ready for review. Lmk if you've got any feedback