Skip to content

Conversation

@dgdavid
Copy link
Contributor

@dgdavid dgdavid commented Apr 14, 2025

Problem

@ancorgs detected and reported some not translatable strings at HostnamePage.tsx

<ancorgs> dgdavid: retrospective code review. These strings are not translatable https://github.com/agama-project/agama/blob/master/web/src/components/system/HostnamePage.tsx#L66

Solution

Make them translatable and take the opportunity for avoid the usage of "please" word according to SUSE documentation style guide, https://documentation.suse.com/style/current/single-html/docu_styleguide/index.html#sec-vocabulary

Notes

A new setRequestError has been added for simplicity, since it make no sense to render something like

Check the following before continuing

Hostname could not be updated.

Even when such an error will likely not happen. So, better to go for

Hostname could not be updated

The presence or not presence of the dot at the end of the sentence it is also intentional: it's not used in titles

Of course, it is needed a better error reporting for forms to easily distinguish between validation warnings and request errors, but sadly Agama is not there at the time of sending this hotfix to make possible the translation of these texts. Thus, skipping further improvements in this form until such a goal can be achieved.

Since they were not. Also take the opportunity for dropping "please"
usage according to SUSE documentation style guide.

https://documentation.suse.com/style/current/single-html/docu_styleguide/index.html#sec-vocabulary

<Form id="hostnameForm" onSubmit={submit}>
{success && <Alert variant="success" isInline title={success} />}
{requestError && <Alert variant="warning" isInline title={requestError} />}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not using error here, which would be more precise, because it is not being used in other forms at the time of writing. As said in the PR description, we're aware that better form validation and error reporting is needed but they should be addresses all together once we find a way to do so.

Copy link
Contributor

@ancorgs ancorgs left a comment

Choose a reason for hiding this comment

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

LGTM

@dgdavid dgdavid merged commit 17a34e6 into master Apr 14, 2025
7 checks passed
@dgdavid dgdavid deleted the fix-not-translatable-strings branch April 14, 2025 11:07
@imobachgs imobachgs mentioned this pull request Apr 22, 2025
imobachgs added a commit that referenced this pull request Apr 22, 2025
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