Skip to content

Commit

Permalink
Fix the time displayed in the tray icon
Browse files Browse the repository at this point in the history
- Take the break notification time into account
- Move the timeToNextBreak logic into a separate getter that is used everywhere this time is displayed
- Use rounded time for the tray icon (to align with the tooltip)
  • Loading branch information
Wikiwix committed Nov 2, 2024
1 parent a790f12 commit f32b922
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 35 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

### Fixed
- error when end break shortcut is not set
- time in tray shows the correct number (and matches the tooltip value)

### Changed
- improved break window loading
Expand Down
17 changes: 17 additions & 0 deletions app/breaksPlanner.js
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,23 @@ class BreaksPlanner extends EventEmitter {
}
}
}

get timeToNextBreak () {
if (this.scheduler.reference === 'startMicrobreak' || this.scheduler.reference === 'startBreak') {
return this.scheduler.timeLeft

Check warning on line 276 in app/breaksPlanner.js

View check run for this annotation

Codecov / codecov/patch

app/breaksPlanner.js#L275-L276

Added lines #L275 - L276 were not covered by tests
}
if (this.scheduler.reference === 'startBreakNotification') {
return this.scheduler.timeLeft + (this.settings.get('breakNotification')

Check warning on line 279 in app/breaksPlanner.js

View check run for this annotation

Codecov / codecov/patch

app/breaksPlanner.js#L278-L279

Added lines #L278 - L279 were not covered by tests
? this.settings.get('breakNotificationInterval')
: 0)
}
if (this.scheduler.reference === 'startMicrobreakNotification') {
return this.scheduler.timeLeft + (this.settings.get('microbreakNotification')

Check warning on line 284 in app/breaksPlanner.js

View check run for this annotation

Codecov / codecov/patch

app/breaksPlanner.js#L283-L284

Added lines #L283 - L284 were not covered by tests
? this.settings.get('microbreakNotificationInterval')
: 0)
}
return null

Check warning on line 288 in app/breaksPlanner.js

View check run for this annotation

Codecov / codecov/patch

app/breaksPlanner.js#L288

Added line #L288 was not covered by tests
}
}

module.exports = BreaksPlanner
2 changes: 1 addition & 1 deletion app/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ function trayIconPath () {
darkMode: nativeTheme.shouldUseDarkColors,
platform: process.platform,
timeToBreakInTray: settings.get('timeToBreakInTray'),
timeToBreak: Utils.minutesRemaining(breakPlanner.scheduler.timeLeft),
timeToBreak: Utils.minutesRemaining(breakPlanner.timeToNextBreak),
reference: breakPlanner.scheduler.reference
}
const trayIconFileName = new AppIcon(params).trayIconFileName
Expand Down
2 changes: 1 addition & 1 deletion app/utils/appIcon.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class AppIcon {
const invertedMonochromeString = this.inverted ? 'Inverted' : ''
const darkModeString = this.darkMode ? 'Dark' : ''
const timeToBreakInTrayString = (this.paused || this.reference === 'finishMicrobreak' ||
this.reference === 'finishBreak' || !this.timeToBreakInTray)
this.reference === 'finishBreak' || !this.timeToBreakInTray || !Number.isInteger(this.timeToBreak))
? ''
: `Number${this.timeToBreak}`

Expand Down
31 changes: 5 additions & 26 deletions app/utils/statusMessages.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ class StatusMessages {
this.doNotDisturb = breakPlanner.dndManager.isOnDnd
this.appExclusionPause = breakPlanner.appExclusionsManager.isSchedulerCleared
this.timeLeft = breakPlanner.scheduler.timeLeft
this.timeToNextBreak = breakPlanner.timeToNextBreak

Check warning on line 10 in app/utils/statusMessages.js

View check run for this annotation

Codecov / codecov/patch

app/utils/statusMessages.js#L10

Added line #L10 was not covered by tests
this.isPaused = breakPlanner.isPaused
this.breakNumber = breakPlanner.breakNumber
this.settings = settings
Expand Down Expand Up @@ -43,38 +44,16 @@ class StatusMessages {

const breakInterval = this.settings.get('breakInterval') + 1
const breakNumber = this.breakNumber % breakInterval
const breakNotificationInterval = this.settings.get('breakNotification')
? this.settings.get('breakNotificationInterval')
: 0
const microbreakNotificationInterval = this.settings.get('microbreakNotification')
? this.settings.get('microbreakNotificationInterval')
: 0

if (this.reference === 'startBreak') {
if (this.reference === 'startBreak' || this.reference === 'startBreakNotification') {

Check warning on line 48 in app/utils/statusMessages.js

View check run for this annotation

Codecov / codecov/patch

app/utils/statusMessages.js#L48

Added line #L48 was not covered by tests
message += i18next.t('statusMessages.nextLongBreak') + ' ' +
Utils.formatTimeIn(this.timeLeft, this.settings.get('language'))
Utils.formatTimeIn(this.timeToNextBreak, this.settings.get('language'))
return message
}

if (this.reference === 'startMicrobreak') {
if (this.reference === 'startMicrobreak' || this.reference === 'startMicrobreakNotification') {

Check warning on line 54 in app/utils/statusMessages.js

View check run for this annotation

Codecov / codecov/patch

app/utils/statusMessages.js#L54

Added line #L54 was not covered by tests
message += i18next.t('statusMessages.nextMiniBreak') + ' ' +
Utils.formatTimeIn(this.timeLeft, this.settings.get('language'))
if (this.settings.get('break')) {
message += '\n' + i18next.t('statusMessages.nextLongBreak') + ' ' +
i18next.t('statusMessages.afterMiniBreak', { count: breakInterval - breakNumber })
}
return message
}

if (this.reference === 'startBreakNotification') {
message += i18next.t('statusMessages.nextLongBreak') + ' ' +
Utils.formatTimeIn(this.timeLeft + breakNotificationInterval, this.settings.get('language'))
return message
}

if (this.reference === 'startMicrobreakNotification') {
message += i18next.t('statusMessages.nextMiniBreak') + ' ' +
Utils.formatTimeIn(this.timeLeft + microbreakNotificationInterval, this.settings.get('language'))
Utils.formatTimeIn(this.timeToNextBreak, this.settings.get('language'))
if (this.settings.get('break')) {
message += '\n' + i18next.t('statusMessages.nextLongBreak') + ' ' +
i18next.t('statusMessages.afterMiniBreak', { count: breakInterval - breakNumber })
Expand Down
4 changes: 1 addition & 3 deletions app/utils/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,7 @@ function formatKeyboardShortcut (keyboardShortcut) {
}

function minutesRemaining (milliseconds) {
const seconds = Math.ceil(milliseconds / 1000.0)
const minutes = Math.ceil(seconds / 60.0)
return minutes
return Math.round(milliseconds / 60000.0)
}

function shouldShowNotificationTitle (platform, systemVersion) {
Expand Down
11 changes: 7 additions & 4 deletions test/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,16 @@ describe('canSkip and canPostpone', () => {

describe('minutesRemaining', () => {
it('one minute remaining', () => {
minutesRemaining(60000).should.equal(1)
minutesRemaining(60 * 1000).should.equal(1)
})
it('less then one minute remaining', () => {
minutesRemaining(1).should.equal(1)
it('twenty seconds remaining', () => {
minutesRemaining(20 * 1000).should.equal(0)
})
it('forty seconds remaining', () => {
minutesRemaining(40 * 1000).should.equal(1)
})
it('ten minutes remaining', () => {
minutesRemaining(600000).should.equal(10)
minutesRemaining(600 * 1000).should.equal(10)
})
})

Expand Down

0 comments on commit f32b922

Please sign in to comment.