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

Desktop: Seamless-Updates - creation of update notification #10791

Merged
merged 5 commits into from
Aug 8, 2024

Conversation

AliceHincu
Copy link
Contributor

This commit contains:

  • the update notification and its style
  • the addition of the update flag
  • fixed linux check

@laurent22
Copy link
Owner

Would you mind providing a screenshot of the notification?


const messageHtml = `
<div class="update-notification" style="color: ${theme.color2};">
A new update (${info.version}) is available. <a href="#" onclick="openChangelogLink()" style="color: ${theme.color2};">See Changelog</a>
Copy link
Owner

Choose a reason for hiding this comment

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

Please localise all user-facing text. Here for example if would be _('A new update (%s) is available', info.version)

@AliceHincu
Copy link
Contributor Author

Sure! This is how it looks like:
image

Comment on lines 61 to 64
${_('A new update (%s) is available', info.version)} <a href="#" onclick="openChangelogLink()" style="color: ${theme.color2};">${_('See Changelog')}</a>
<div style="display: flex; gap: 10px; margin-top: 8px;">
<button onclick="document.dispatchEvent(new CustomEvent('${UpdateNotificationEvents.ApplyUpdate}'))" class="notyf__button notyf__button--confirm" style="color: ${theme.color2};">${_('Restart now')}</button>
<button onclick="document.dispatchEvent(new CustomEvent('${UpdateNotificationEvents.Dismiss}'))" class="notyf__button notyf__button--dismiss" style="color: ${theme.color2};">${_('Update Later')}</button>
Copy link
Owner

Choose a reason for hiding this comment

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

Please make sure you escape the translated strings (otherwise it'd be possible for a translator to inject html in here). See here for more information: https://joplinapp.org/help/dev/coding_style/#escape-variables


const messageHtml = `
<div class="update-notification" style="color: ${theme.color2};">
${_('A new update (%s) is available', info.version)} <a href="#" onclick="openChangelogLink()" style="color: ${theme.color2};">${_('See Changelog')}</a>
Copy link
Owner

Choose a reason for hiding this comment

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

"See Changelog" => "See changelog" (we don't use titlecase in general)

${_('A new update (%s) is available', info.version)} <a href="#" onclick="openChangelogLink()" style="color: ${theme.color2};">${_('See Changelog')}</a>
<div style="display: flex; gap: 10px; margin-top: 8px;">
<button onclick="document.dispatchEvent(new CustomEvent('${UpdateNotificationEvents.ApplyUpdate}'))" class="notyf__button notyf__button--confirm" style="color: ${theme.color2};">${_('Restart now')}</button>
<button onclick="document.dispatchEvent(new CustomEvent('${UpdateNotificationEvents.Dismiss}'))" class="notyf__button notyf__button--dismiss" style="color: ${theme.color2};">${_('Update Later')}</button>
Copy link
Owner

Choose a reason for hiding this comment

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

Update Later => Update later

Dismiss = 'dismiss-update-notification',
}

const changelogLink = 'https://joplinapp.org/help/about/changelog/desktop/';
Copy link
Owner

Choose a reason for hiding this comment

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

For the changelog it's a bit tricky because it is updated with a delay, so it may not have the information by the time the user gets the notification. So to be sure let's link instead to https://github.com/laurent22/joplin/releases

@laurent22 laurent22 merged commit 88b3c7f into laurent22:dev Aug 8, 2024
10 checks passed
@AliceHincu AliceHincu deleted the seamless-updates branch August 10, 2024 13:45
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.

2 participants