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

fix(desktop): change "reload page" messages to "restart app" #14181

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Jan 23, 2025

☑️ Resolves

  • On Talk Desktop it shouldn't say, "Reload the page". Instead, it must be "Restart the app".
  • Module name might not be the best, but rushing before the string freeze to have the strings. Module/structure can be refactored afterwards.

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

image

image

image

🏁 Checklist

  • 🌏 Tested with different browsers / clients:
    • Chromium (Chrome / Edge / Opera / Brave)
    • Firefox
    • Safari
    • Talk Desktop
    • Not risky to browser differences / client
  • 🖌️ Design was reviewed, approved or inspired by the design team
  • ⛑️ Tests are included or not possible
  • 📗 User documentation in https://github.com/nextcloud/documentation/tree/master/user_manual/talk has been updated or is not required

@@ -136,7 +137,7 @@ export async function setRemoteCapabilities(joinRoomResponse: JoinRoomFullRespon
patchTokenMap(joinRoomResponse.data.ocs.data)

// As normal capabilities update, requires a reload to take effect
showError(t('spreed', 'Nextcloud Talk Federation was updated, please reload the page'), {
showError(t('spreed', 'Nextcloud Talk Federation was updated.') + '\n' + messagePleaseReload, {
Copy link
Member

Choose a reason for hiding this comment

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

Something for later, but this should say something like:

"Nextcloud Talk was updated on the federated server."

And we should have a look if we can make sure it does not yield when you open such a conversation the first time

Copy link
Member

Choose a reason for hiding this comment

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

Extracted to #14201

Copy link
Contributor

Choose a reason for hiding this comment

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

Another point regarding \n: we discussed with Dorra, if toasts width should be limited upstream to ~750px (80 characters per line is recommended by W3C standard)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if toasts width should be limited

If a toast contains a long complex sentence, it probably shouldn't be a toast, but a part of UI

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
@ShGKme ShGKme force-pushed the desktop/reload-the-page-messages branch from 0707fb4 to 5e9eaca Compare January 23, 2025 15:45
@ShGKme
Copy link
Contributor Author

ShGKme commented Jan 23, 2025

  • Rebased onto CI fix
  • Tested only on Desktop

@ShGKme ShGKme marked this pull request as ready for review January 23, 2025 15:46
@nickvergessen
Copy link
Member

/backport! to stable31

@nickvergessen nickvergessen merged commit 7e7e699 into main Jan 23, 2025
52 checks passed
@nickvergessen nickvergessen deleted the desktop/reload-the-page-messages branch January 23, 2025 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants