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

feat: Add printer url in search params so that URLs become bookmarkable #1574

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

andrewboktor
Copy link

@andrewboktor andrewboktor commented Jan 14, 2025

Add printer url in search params so that URLs become bookmarkable
Look at search param while loading and use that to find the right config

Resolves #677

Signed-off-by: Andrew Boktor [email protected]

andrewboktor and others added 2 commits January 13, 2025 21:42
Look at search param while loading and use that to find the right config
@pedrolamas
Copy link
Member

Hi @andrewboktor, first of all, a big THANK YOU for this PR!

I admit I have been working on this for a few months now and my solution was to use the VueRouter to do it, only I was hitting a lot of in-app navigation issues and so I just stopped...

Your approach to altering the main URL (the part before the '#' navigation) is the solution for that problem!

I will be taking this PR as base and merging a few changes (I want to try and ensure that page is not reloaded as it currently does, and am considering replacing the btoa with an MD5 hash instead), but it will definitely be based on the work you have provided here! 😀

@pedrolamas pedrolamas self-assigned this Jan 14, 2025
@pedrolamas pedrolamas added the FR - Enhancement New feature or request label Jan 14, 2025
@pedrolamas pedrolamas added this to the 1.31.5 milestone Jan 14, 2025
@pedrolamas pedrolamas removed their assignment Jan 14, 2025
@andrewboktor
Copy link
Author

Hi @andrewboktor, first of all, a big THANK YOU for this PR!

I admit I have been working on this for a few months now and my solution was to use the VueRouter to do it, only I was hitting a lot of in-app navigation issues and so I just stopped...

Your approach to altering the main URL (the part before the '#' navigation) is the solution for that problem!

I will be taking this PR as base and merging a few changes (I want to try and ensure that page is not reloaded as it currently does, and am considering replacing the btoa with an MD5 hash instead), but it will definitely be based on the work you have provided here! 😀

Yea MD5 hash works too, or any other hash for that matter if we want to make the URLs smaller.
Another alternative I considered was adding an ID to ApiConfig when it's stored in localstorage, but apiUrl works just as well as an ID so I ditched that idea.

I do agree that likely the cleanest solution is to add a "printer" as part of the app paths in the router to all the routes. But didn't go that direction because this likely would require touching a lot of code that I'm not familiar with.

Please let me know if I can yield any assistance, bounce ideas, or prototype some solution or other.
Happy to test if you have a branch as well.

@andrewboktor
Copy link
Author

I pulled the branch and ran a quick test and it seems to be working well!
May be an improvement here is to remove the printer from search params if it's not found in localStorage.

@andrewboktor
Copy link
Author

I've been using this on my printer server for about 2 days and it's working as expected

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FR - Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bookmarks in browser for multiple printers
2 participants