Skip to content

Show ip unavailble error message on desktop#7987

Merged
Pururun merged 1 commit intomainfrom
show-ip-version-error-desktop
Apr 10, 2025
Merged

Show ip unavailble error message on desktop#7987
Pururun merged 1 commit intomainfrom
show-ip-version-error-desktop

Conversation

@Pururun
Copy link
Copy Markdown
Contributor

@Pururun Pururun commented Apr 9, 2025

This adds support for the same error message to the desktop frontend as was introduced on android in the following PR:
#7976


This change is Reviewable

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

Files not reviewed (1)
  • desktop/packages/mullvad-vpn/locales/messages.pot: Language not supported

Comment on lines 236 to 250
case TunnelParameterError.ipv6Unavailable:
return messages.pgettext(
'notifications',
'IPv6 is not available',
);
}
Copy link

Copilot AI Apr 9, 2025

Choose a reason for hiding this comment

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

The 'case TunnelParameterError.ipv6Unavailable:' statement is misaligned relative to the previous case, which may lead to confusion. Consider aligning it with the other case labels to improve readability and maintain consistency.

Suggested change
case TunnelParameterError.ipv6Unavailable:
return messages.pgettext(
'notifications',
'IPv6 is not available',
);
}
case TunnelParameterError.ipv6Unavailable:
return messages.pgettext(
'notifications',
'IPv6 is not available',
);

Copilot uses AI. Check for mistakes.
tobias-jarvelov
tobias-jarvelov previously approved these changes Apr 9, 2025
Copy link
Copy Markdown
Contributor

@tobias-jarvelov tobias-jarvelov left a comment

Choose a reason for hiding this comment

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

We could add translators comments as we strive to add those to all translations in the desktop app. Other than that it :lgtm:

Reviewed 3 of 4 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Pururun)


desktop/packages/mullvad-vpn/src/shared/notifications/error.ts line 234 at r2 (raw file):

        return messages.pgettext('notifications', 'IPv4 is not available');
      case TunnelParameterError.ipv6Unavailable:
        return messages.pgettext('notifications', 'IPv6 is not available');

suggestion: We could add TRANSLATORS comments here to aid the crowdin translators:

case TunnelParameterError.ipv4Unavailable:
  // TRANSLATORS: Label for notification when IPv4 is not available.
  return messages.pgettext('notifications', 'IPv4 is not available');
case TunnelParameterError.ipv6Unavailable:
  // TRANSLATORS: Label for notification when IPv6 is not available.
  return messages.pgettext('notifications', 'IPv6 is not available');

Code quote:

      case TunnelParameterError.ipv4Unavailable:
        return messages.pgettext('notifications', 'IPv4 is not available');
      case TunnelParameterError.ipv6Unavailable:
        return messages.pgettext('notifications', 'IPv6 is not available');

@Pururun
Copy link
Copy Markdown
Contributor Author

Pururun commented Apr 10, 2025

@tobias-jarvelov Please check again

Copy link
Copy Markdown
Contributor Author

@Pururun Pururun left a comment

Choose a reason for hiding this comment

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

Reviewable status: 2 of 5 files reviewed, 1 unresolved discussion (waiting on @tobias-jarvelov)


desktop/packages/mullvad-vpn/src/shared/notifications/error.ts line 234 at r2 (raw file):

Previously, tobias-jarvelov (Tobias Järvelöv) wrote…

suggestion: We could add TRANSLATORS comments here to aid the crowdin translators:

case TunnelParameterError.ipv4Unavailable:
  // TRANSLATORS: Label for notification when IPv4 is not available.
  return messages.pgettext('notifications', 'IPv4 is not available');
case TunnelParameterError.ipv6Unavailable:
  // TRANSLATORS: Label for notification when IPv6 is not available.
  return messages.pgettext('notifications', 'IPv6 is not available');

Added

Copy link
Copy Markdown
Contributor

@tobias-jarvelov tobias-jarvelov left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@Pururun Pururun force-pushed the show-ip-version-error-desktop branch from f7798e8 to 120d29e Compare April 10, 2025 15:26
@Pururun Pururun merged commit 1c5712f into main Apr 10, 2025
28 checks passed
@Pururun Pururun deleted the show-ip-version-error-desktop branch April 10, 2025 15: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.

3 participants