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 name to browser tab while printing or complete #1371

Merged

Conversation

mikebcbc
Copy link
Contributor

@mikebcbc mikebcbc commented May 3, 2023

Adds the printer name to the browser tab during printing and complete statuses.

Description

This PR addresses #1322 and adds the printer name to the browser tab while the printer is printing, or when the print is complete.

Related Tickets & Documents

#1322

Mobile & Desktop Screenshots/Recordings

image

[optional] Are there any post-deployment tasks we need to perform?

No post-deployment tasks.

Adds the printer name to the browser tab during printing and complete statuses.
@github-actions
Copy link
Contributor

github-actions bot commented May 3, 2023

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 0

src/components/settings/SettingsUiSettingsTab.vue Outdated Show resolved Hide resolved
src/locales/en.json Outdated Show resolved Hide resolved
@meteyou
Copy link
Member

meteyou commented May 6, 2023

Thx for your PR. I have only one question about it. What happens when the user don't set a printername in the settings?

@github-actions
Copy link
Contributor

github-actions bot commented May 7, 2023

Language file analysis report:

File Missing Keys Unused Keys
da.json 6 0
de.json 2 0
en.json 0 0
ko.json 146 2

@mikebcbc
Copy link
Contributor Author

mikebcbc commented May 7, 2023

Thx for your PR. I have only one question about it. What happens when the user don't set a printername in the settings?

@meteyou - good question, I updated this PR to handle that. Before it would just show the hyphen, but now I am conditionally showing the hyphen. Take a look at the PR. Here's two examples:

printername defined:
image

printername not defined:

image

@mikebcbc mikebcbc requested a review from meteyou May 7, 2023 19:55
@github-actions
Copy link
Contributor

github-actions bot commented May 7, 2023

Language file analysis report:

File Missing Keys Unused Keys
da.json 6 0
de.json 2 0
en.json 0 0
ko.json 146 2

@github-actions
Copy link
Contributor

github-actions bot commented May 7, 2023

Language file analysis report:

File Missing Keys Unused Keys
da.json 6 0
de.json 2 0
en.json 0 0
ko.json 146 2

@meteyou
Copy link
Member

meteyou commented May 7, 2023

hmm... maybe my "idea" with adding the {name} in the translation itself was wrong for this use case. maybe you should just use getter to add " - ", when it exists and we also don't have an issue with the "new" translation keys.

@github-actions
Copy link
Contributor

github-actions bot commented May 7, 2023

Language file analysis report:

File Missing Keys Unused Keys
da.json 9 3
de.json 5 3
en.json 0 0
ko.json 149 5

@mikebcbc
Copy link
Contributor Author

mikebcbc commented May 7, 2023

hmm... maybe my "idea" with adding the {name} in the translation itself was wrong for this use case. maybe you should just use getter to add " - ", when it exists and we also don't have an issue with the "new" translation keys.

@meteyou Are you saying to force it in the getter? Like this:

return i18n.t('App.Titles.PrintingETA', {
                    percent: percent,
                    filename: state.printer?.print_stats?.filename,
                    eta
                }) + state.gui?.general.printername ? `- ${state.gui?.general.printername}` : ""

If so, I feel like that's not very clean... I feel like it would be cleaner to just supply the - {name} and update the translation keys. If we don't want to worry about translation, since it's all the same no matter the language I can just update every translation and append this where it fits... what are you thoughts on that?

@meteyou
Copy link
Member

meteyou commented May 7, 2023

i would use write it a little bit different (just for readability):

let output = i18n.t('App.Titles.PrintingETA', {
	                    percent: percent,
	                    filename: state.printer?.print_stats?.filename,
	                    eta
	                })

// add printer name if it exists
if (state.gui?.general.printername) output += ` - ${state.gui.general.printername}`

return output

I just don't like to add "decorators" in a translation variable.

just my 2 cents about this topic...
(my solution have not to be the best way... its just my style to do it...)

@mikebcbc
Copy link
Contributor Author

mikebcbc commented May 8, 2023

@

i would use write it a little bit different (just for readability):

let output = i18n.t('App.Titles.PrintingETA', {
	                    percent: percent,
	                    filename: state.printer?.print_stats?.filename,
	                    eta
	                })

// add printer name if it exists
if (state.gui?.general.printername) output += ` - ${state.gui.general.printername}`

return output

I just don't like to add "decorators" in a translation variable.

just my 2 cents about this topic... (my solution have not to be the best way... its just my style to do it...)

@meteyou - I think that makes sense. I think it does all come down to style - looks clean to me. I updated this PR and removed it from the translation keys. Let me know what you think! Thanks for the clarification!

@mikebcbc
Copy link
Contributor Author

@meteyou - anything else you need from me on this PR, or is it good to go?

@meteyou
Copy link
Member

meteyou commented May 12, 2023

LGTM! Thanks!

@meteyou meteyou merged commit 140d796 into mainsail-crew:develop May 12, 2023
@mikebcbc mikebcbc deleted the feature/printer-name-in-browser-tab branch May 12, 2023 15:08
@wikolii
Copy link

wikolii commented May 9, 2024

Hi, is there any way one could choose to have the machine name in front of the tab description?
In my case I normally just print the same part in many printers and having to wait for the popup with the full tab description to know the machine name is time consuming.

I would even choose not to have the filename on the tab description, maybe there could be some setting for that.
Thanks!

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.

3 participants