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

[sailfish-utilities] Use DSO as target for more info links. JB#59782 #63

Merged
merged 2 commits into from Jan 13, 2023

Conversation

vigejolla
Copy link
Member

TJC is in read only mode, in DSO we can update the docs.

Copy link
Member

@llewelld llewelld left a comment

Choose a reason for hiding this comment

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

The new links are much nicer: relevant and more professional looking too, so these changes make perfect sense.

It's just a thought but I wonder if it would make sense to pass the links to the qsTrId as a parameter, to avoid having to redo all translations if the location changes in future.

Either way, this looks good to me.

@vigejolla
Copy link
Member Author

@sledges what's your take on @llewelld 's suggestion above? I can still change the PR if you think that would make life easier in the future.

@sledges
Copy link

sledges commented Jan 12, 2023

It certainly makes sense to externalise HTML tags and URL, not only to reduce burden per-language, but also to prevent any little Bobby Tables cases:)

Here's an example of "extra comment" of how I'd suggest to implement this: https://translate.sailfishos.org/en_GB/jolla-settings-system/translate/settings-system.ts#unit=437942

While our extra comment would read:
Text surrounded by %1 and %2 becomes a hyperlink: %1 is replaced by <a href="..."> and %2 by </a>.

Copy link
Member

@llewelld llewelld left a comment

Choose a reason for hiding this comment

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

Thanks vigejolla. Although you didn't quite follow sledges suggested wording, I personally think the wording you've used is just fine.

Just one small change needed in order to make the tracker link work, but otherwise this looks good to me.

qml/plugins/CleanTracker.qml Outdated Show resolved Hide resolved
…. JB#59782

Signed-off-by: Ville Nummela <ville.nummela@jolla.com>
@vigejolla
Copy link
Member Author

Changed also the text ID to prevent pootle from wiping out old translations

Copy link

@sledges sledges left a comment

Choose a reason for hiding this comment

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

🙇

@sledges
Copy link

sledges commented Jan 13, 2023

P. S.: Should one commit have "Fixes" for the bug?

Copy link
Member

@llewelld llewelld left a comment

Choose a reason for hiding this comment

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

Looks great to me and works nicely. The updated links are definitely a nice improvement.

…59782

TJC is in read only mode, in DSO we can update the docs.

Signed-off-by: Ville Nummela <ville.nummela@jolla.com>
@vigejolla
Copy link
Member Author

P. S.: Should one commit have "Fixes" for the bug?

Yes. I believe you can guess my reasoning behind the decision to leave it out, and also now adding it after you pointed it out 😄

@vigejolla vigejolla merged commit 1069324 into master Jan 13, 2023
@vigejolla vigejolla deleted the jb59782 branch January 13, 2023 08:28
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.

None yet

3 participants