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.

Sign up for GitHub

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

Jump to bottom

Translate severity levels and module names in result page #463

Merged
merged 6 commits into from
Jun 11, 2024
Merged

Translate severity levels and module names in result page #463

merged 6 commits into from
Jun 11, 2024

Conversation

hannaeko
Copy link
Member

Purpose

Translate severity levels and module names

Context

Fixes #86

Changes

  • Translate severity levels
  • Translate modules names

How to test this PR

  • Load a test result in a language that is not English and have the translation done
  • Check that the module names are translated
  • Check that the tooltip of the severity levels in module and test case heading icon are translated
  • Check that the severity levels in test case messages are translated

@hannaeko hannaeko linked an issue Feb 27, 2024 that may be closed by this pull request
Translation of categories, type of errors... #86
Closed
@hannaeko hannaeko added this to the v2024.1 milestone Feb 27, 2024
@hannaeko hannaeko changed the title translate severity level names in result page Translate severity level and module names in result page Feb 27, 2024
@hannaeko hannaeko changed the title Translate severity level and module names in result page Translate severity levels and module names in result page Feb 27, 2024
@hannaeko hannaeko added the V-Minor Versioning: The change gives an update of minor in version. label Feb 29, 2024
marc-vanderwal
marc-vanderwal previously approved these changes Mar 6, 2024
View reviewed changes
tgreenx
tgreenx previously approved these changes May 14, 2024
View reviewed changes
Copy link
Contributor

@tgreenx tgreenx left a comment

Choose a reason for hiding this comment

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

Tested with the 'fr' locale and it works as expected.

matsduf
matsduf reviewed May 24, 2024
View reviewed changes
src/app/components/result/result.component.ts
Comment on lines +44 to +53
'system': $localize `System`,
'basic': $localize `Basic`,
'address': $localize `Address`,
'connectivity': $localize `Connectivity`,
'consistency': $localize `Consistency`,
'delegation': $localize `Delegation`,
'dnssec': $localize `DNSSEC`,
'nameserver': $localize `Nameserver`,
'syntax': $localize `Syntax`,
'zone': $localize `Zone`,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a list of the current modules. What is a new module name comes, e.g. the name of a custom module?

Copy link
Contributor

Choose a reason for hiding this comment

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

That’s a limitation of the current design. Shouldn’t these module names be moved over to Zonemaster::Engine, eventually?

Copy link
Contributor

Choose a reason for hiding this comment

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

If custom module names just show up untranslated, it is fine. If they disappear, it is not fine

I think we should consider moving it to Engine later but it is fine to have it in GUI for now.

matsduf
matsduf previously approved these changes May 24, 2024
View reviewed changes
Copy link
Contributor

@matsduf matsduf left a comment

Choose a reason for hiding this comment

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

My approval is based on the assumption that names of custom modules are just untranslated, and not creating any other issue

@hannaeko hannaeko dismissed stale reviews from matsduf, tgreenx, and marc-vanderwal via cee19dc May 30, 2024 13:56
@hannaeko
Copy link
Member Author

hannaeko commented May 30, 2024 •
edited
Loading

My approval is based on the assumption that names of custom modules are just untranslated, and not creating any other issue

This was not the case and should now be fixed.

That’s a limitation of the current design. Shouldn’t these module names be moved over to Zonemaster::Engine, eventually?

I implemented this based on the decision taken in #86 to only translate in Gui but I guess this issue is a bit old now and things have changed since ^^

@matsduf
Copy link
Contributor

matsduf commented May 31, 2024

I implemented this based on the decision taken in #86 to only translate in Gui but I guess this issue is a bit old now and things have changed since ^^

I think that is fine for now. We can take the next step later.

@matsduf
Copy link
Contributor

matsduf commented Jun 3, 2024

@tgreenx and @marc-vanderwal, please re-review.

@tgreenx tgreenx merged commit 0c95e9f into zonemaster:develop Jun 11, 2024
1 check passed
@tgreenx tgreenx added the S-ReleaseTested Status: The PR has been successfully tested in release testing label Jun 20, 2024
@tgreenx
Copy link
Contributor

tgreenx commented Jun 20, 2024

v2024.1 Release testing

Followed the "How to Test" section, using the fr locale, and it works as advertised.

@tgreenx tgreenx mentioned this pull request Jun 20, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matsduf matsduf matsduf approved these changes

@tgreenx tgreenx tgreenx approved these changes

@marc-vanderwal marc-vanderwal marc-vanderwal approved these changes

@mattias-p mattias-p Awaiting requested review from mattias-p

Assignees
No one assigned
Labels
S-ReleaseTested Status: The PR has been successfully tested in release testing V-Minor Versioning: The change gives an update of minor in version.
Projects
None yet
Milestone
v2024.1
Development

Successfully merging this pull request may close these issues.

Translation of categories, type of errors...
4 participants
@hannaeko @matsduf @tgreenx @marc-vanderwal